Skip to content

HLint: Pass options through user config #1724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Apr 21, 2021
Merged

HLint: Pass options through user config #1724

merged 11 commits into from
Apr 21, 2021

Conversation

rmehri01
Copy link
Contributor

@rmehri01 rmehri01 commented Apr 14, 2021

Closes #1509.

I actually got it to work on a sample file using:

"flags": [
    "-iUnused LANGUAGE pragma",
    "-h/Users/ryanmehri/my-hlint-config.yaml"
]

I think the fact that the command line arguments were different than the library ones just threw me off! I'll be working on adding the tests soon.

@jneira jneira self-requested a review April 14, 2021 05:24
@rmehri01 rmehri01 marked this pull request as ready for review April 15, 2021 20:08
@ndmitchell
Copy link
Collaborator

I'd have expected that all the config options are documented somewhere. If so, this should be added.

@jneira
Copy link
Member

jneira commented Apr 19, 2021

Hi, many thanks for your work, the plugin will have a very handy and flexible config option (maybe it will not need no one more 😄)
Maybe to have a positive test would be interesting: starting with no hlint hints, change the config and then check the expected new hint is present, @rmehri01 what do you think?

@rmehri01
Copy link
Contributor Author

Maybe to have a positive test would be interesting: starting with no hlint hints, change the config and then check the expected new hint is present

Sounds good, I'll add it!

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, many thanks again

@jneira jneira added the merge me Label to trigger pull request merge label Apr 20, 2021
@berberman berberman removed the merge me Label to trigger pull request merge label Apr 20, 2021
@jneira
Copy link
Member

jneira commented Apr 21, 2021

Well i guess this is good to go now
//cc @berberman

@jneira jneira added the merge me Label to trigger pull request merge label Apr 21, 2021
@mergify mergify bot merged commit 369fdfe into haskell:master Apr 21, 2021
@rmehri01 rmehri01 deleted the hlint-options branch April 21, 2021 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible to pass options to HLint?
4 participants