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 make verify target for running static analysis checks #34

Merged
merged 3 commits into from
Apr 2, 2024

Conversation

henrybear327
Copy link
Contributor

@henrybear327 henrybear327 commented Apr 1, 2024

Add Makefile targets for running fmt and static analysis checks

Changes:

  • Introduce make verify and make fmt targets in Makefile
  • For linter, use the same config as etcd repo
  • Update GitHubCI workflow accordingly

Closes #31

References:

@henrybear327
Copy link
Contributor Author

henrybear327 commented Apr 1, 2024

@jmhbnz do we want to update the static-analysis pipeline to use make verify?

Also, would we want to include the fmt part for house-keeping, too? :)

@siyuanfoundation
Copy link
Contributor

@jmhbnz do we want to update the static-analysis pipeline to use make verify?

Also, would we want to include the fmt part for house-keeping, too? :)

I think yes for both :)

Thanks!

@jmhbnz
Copy link
Member

jmhbnz commented Apr 1, 2024

@jmhbnz do we want to update the static-analysis pipeline to use make verify?
Also, would we want to include the fmt part for house-keeping, too? :)

I think yes for both :)

Agreed - Thanks @henrybear327

Note: Please ensure your commit is signed so the developer certificate of origin check passes.

@henrybear327 henrybear327 force-pushed the feat/add_golangci-lint branch from 969260f to 9ab8987 Compare April 2, 2024 15:51
@henrybear327 henrybear327 marked this pull request as draft April 2, 2024 15:55
@henrybear327 henrybear327 force-pushed the feat/add_golangci-lint branch 4 times, most recently from 32d96d1 to 4c53f6f Compare April 2, 2024 16:22
@henrybear327
Copy link
Contributor Author

@jmhbnz do we want to update the static-analysis pipeline to use make verify?
Also, would we want to include the fmt part for house-keeping, too? :)

I think yes for both :)

Agreed - Thanks @henrybear327

Note: Please ensure your commit is signed so the developer certificate of origin check passes.

Commits are now fixed with proper signing! Thanks for pointing it out!

@henrybear327
Copy link
Contributor Author

@jmhbnz do we want to update the static-analysis pipeline to use make verify?
Also, would we want to include the fmt part for house-keeping, too? :)

I think yes for both :)

Thanks!

Thanks for the feedback! :)

I have updated the linter target and added fmt!

For the linter, I took the linter config file from etcd project, as I am trying to keep the consistency across projects :)

@henrybear327 henrybear327 force-pushed the feat/add_golangci-lint branch from 4c53f6f to c00e611 Compare April 2, 2024 16:26
@henrybear327 henrybear327 marked this pull request as ready for review April 2, 2024 16:26
scripts/fix.sh Outdated Show resolved Hide resolved
@siyuanfoundation
Copy link
Contributor

Don't forget to signoff every commit :)

@siyuanfoundation
Copy link
Contributor

One nit comment. Otherwise LGTM.

@henrybear327 henrybear327 force-pushed the feat/add_golangci-lint branch from c00e611 to 60f0e89 Compare April 2, 2024 16:46
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks @henrybear327

@jmhbnz jmhbnz merged commit 29f8611 into etcd-io:master Apr 2, 2024
3 checks passed
@henrybear327 henrybear327 deleted the feat/add_golangci-lint branch April 4, 2024 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add make verify target for running static analysis checks
4 participants