Skip to content
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

Add hlint gh-action ignoring one suggestion. #85

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

philderbeast
Copy link

Automates a check for one of the checklist items for contributions, that hlint . has no hints.

@philderbeast philderbeast requested a review from vrom911 as a code owner June 30, 2022 13:50
.hlint.yaml Outdated
@@ -0,0 +1,2 @@
# Warnings currently triggered by your code
- ignore: {name: "Unused LANGUAGE pragma"} # 1 hint
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to just remove the unused pragma rather than ignore it 🙂

Copy link
Author

Choose a reason for hiding this comment

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

I'd intended to do this in two steps. Add the gh-action that passes then on one or more separate PRs fix lint suggestions while keeping the gh-action in the green.

Copy link
Member

Choose a reason for hiding this comment

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

I would like to not have ignored suggestions even in the meantime, so I prefer to have everything done in one 🙂

Copy link
Author

Choose a reason for hiding this comment

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

My own preference is for having the ignores in there as a TODO list but I did as you asked.

uses: haskell/actions/hlint-run@v2
with:
path: '["src/", "test/"]'
fail-on: suggestion
Copy link
Member

Choose a reason for hiding this comment

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

I think it could be added ass another job to the existing workflow we have for the project 👌🏼

Copy link
Author

@philderbeast philderbeast Sep 15, 2022

Choose a reason for hiding this comment

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

One reason for having a separate action is that hlint runs very quickly (doesn't require a build).

Copy link
Member

Choose a reason for hiding this comment

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

This step could be done before the Build step

Copy link
Author

Choose a reason for hiding this comment

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

I did this but in testing found it gave less feedback. I had to rely on trusting that running hlint locally would work the same as when run in the workflow. This can be seen by comparing hlint and hlint-standalone workflow runs. The CI/hlint job was last triggered when I edited the workflow whereas the hlint-standalone/hlint-standalone job was triggered when I made any change (hlint is lightweight so this doesn't cost much).

@vrom911 vrom911 added the CI label Sep 15, 2022
@philderbeast philderbeast requested a review from vrom911 October 2, 2022 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants