-
Notifications
You must be signed in to change notification settings - Fork 409
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
Use HLint. #1045
Use HLint. #1045
Conversation
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
What would be the advantages/disadvantages of doing this? The one thing I do not want is to have some CI job that can fail if linting fails. As long as this job only provides suggestions, then I'm neutral on this. |
The advantages of using HLint is that it can suggest improvements to code that may be more complicated or expensive than needed. For example, it's helped me catch silly code such as The advantages of using the hlint-scan action for this is that this allows GitHub's code scanning infrastructure to be used to track extant code issues and code issues in pull requests (examples for what they look like are linked in the initial comment for the pull request). Settings for code scanning determine whether these will be considered failed checks. By default, only the most severe errors are considered to be failures, and I don't think any of HLint's built-in hints are set up to be errors; they are at most warnings. If this were to be used as a status check in the future (which this won't be unless certain settings are changed later), then one thing using GitHub's code scanning infrastructure gives us is that only issues in the code modified by a pull request will be considered an issue for the pull request, and existing issues in other parts of the code base will not be considered an issue for the pull request. All the HLint hints disabled by this pull request are triggered by the existing Hakyll code, even though I think some of them should be enabled at some point (e.g., avoiding the gratuitous use of |
I don't really have a strong opinion on this. I feel like common sense and empathy with the reader of the code often lead to better style than rule-based tools like HLint. But as long as this doesn't block PRs, I don't really mind having it either. |
Let's merge this in. Contributors can use HLint's recommendations, or not. Thanks @chungyc ! |
I thought it would be nice to have a linter, so this adds an HLint workflow which uploads its results to GitHub code scanning. If you do not want a linter or prefer other ways to do linting, feel free to close this pull request without merging.
This change disables hints that would trigger for the existing code. Depending on preference, some of them can be enabled later, while others could be permanently disabled. E.g., removing a needless
$
seems a good idea, while I am not a fan of the suggestion to use<&>
with Hakyll code.To see what warnings and suggestions may look like, see https://github.com/haskell-actions/hlint-scan#examples.