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 #70

Merged
merged 2 commits into from
Oct 3, 2022
Merged

Add HLint #70

merged 2 commits into from
Oct 3, 2022

Conversation

blackheaven
Copy link
Contributor

I'm trying to tackle #63

@blackheaven blackheaven requested a review from chshersh as a code owner October 1, 2022 15:20
@chshersh chshersh added ⚗️ dx Dev experience improvements hacktoberfest-accepted https://hacktoberfest.com/participation/ labels Oct 1, 2022
Copy link
Owner

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, @blackheaven!

I have a few questions and suggestions to clarify the proper CI configuration though

.github/workflows/hlint.yml Outdated Show resolved Hide resolved
Comment on lines 15 to 24
- name: 'Set up HLint'
uses: rwe/actions-hlint-setup@v1
with:
version: '3.1.6'

- name: 'Run HLint'
uses: rwe/actions-hlint-run@v2
with:
path: '["src/", "test/"]'
fail-on: warning
Copy link
Owner

Choose a reason for hiding this comment

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

I haven't used rwe/actions-hlint-setup and rwe/actions-hlint-run actions before.

I would prefer to use official Haskell actions under the haskell github org:

But even then, I'm not sure what is the difference between those two actions and a simple job that just downloads the hlint executable and runs it. I used to configure hlint before with this simple setup:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIUC:

  • rwe/actions-hlint-setup deals with the subtleties of the underlying OS (ie. download le exe on windows, and on)
  • rwe/actions-hlint-run only run hlint on the new/changed files

I update the PR according to your feedback, I'm open to new comments/requests.

Copy link
Owner

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

@blackheaven Thanks a lot for the contribution! I agree that we can always improve the action further if we find lacking something but for now we can start with the simplest thing that works 🙂

@chshersh chshersh merged commit 533f270 into chshersh:main Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚗️ dx Dev experience improvements hacktoberfest-accepted https://hacktoberfest.com/participation/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants