-
Notifications
You must be signed in to change notification settings - Fork 434
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
Initial Lint Support #1055
Initial Lint Support #1055
Conversation
Signed-off-by: shubhindia <shubhindia123@gmail.com>
Signed-off-by: shubhindia <shubhindia123@gmail.com>
Signed-off-by: shubhindia <shubhindia123@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work! I've left a couple of suggestions for consideration.
.github/workflows/lint.yml
Outdated
- name: golangci-lint | ||
uses: golangci/golangci-lint-action@v3 | ||
with: | ||
args: --timeout=30m |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
args: --timeout=30m | |
args: --timeout=2m | |
version: v1.54 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will fail with timeout error. Clean lint check takes around 140s on my M1 Pro Macbook. Currently it takes around 6 mins to run in GA ref: https://github.com/shubhindia/komiser/actions/runs/6454534229
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reduced the timeout to 10mins now. It would be much faster with cache, but 10 mins is a reasonable time to run lint I think.
Co-authored-by: Azanul Haque <42029519+Azanul@users.noreply.github.com>
Signed-off-by: shubhindia <shubhindia123@gmail.com>
874dcd5
to
8d754f7
Compare
Problem
There are no lint checks in place currently.
Solution
This PR adds golangci-lint support. The lint checks are done via github-actions only.
Changes Made
How to Test
golangci-lint run
komiser start --config=config.toml
Checklist