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

chore: add lint and fmt Makefile tasks #641

Merged
merged 3 commits into from
Aug 22, 2022

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Aug 19, 2022

Inspired by https://github.com/celestiaorg/celestia-node/blob/main/Makefile#L61-L74

Testing

I intentionally introduced a markdownlint violation

$ make lint
--> Running linter
WARN [linters context] structcheck is disabled because of go1.18. You can track the evolution of the go1.18 support by following the https://github.com/golangci/golangci-lint/issues/2649.
README.md:62 MD025/single-title/single-h1 Multiple top-level headings in the same document [Context: "# Carrers"]
make: *** [lint] Error 1

@rootulp rootulp self-assigned this Aug 19, 2022
evan-forbes
evan-forbes previously approved these changes Aug 19, 2022
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

hell yeah how have we been living without this!!

do we want to add actually calling gofumpt -w as well somewhere? I'd be fine with either not adding it, or maybe have some sort of differentiation between the commands that actually modify code and others that just let us know something is wrong. Either way 👍 👍

@rootulp
Copy link
Collaborator Author

rootulp commented Aug 19, 2022

I agree, running linters with --fix would be great and we should differentiate between lint check and lint fix. Proposal to run:

  • lint --fix on make fmt
  • lint check on make lint

Motivation: conforms to what celestia-node does here.

Note: gofumpt will be run by golangci-lint because

- gofumpt

@rootulp rootulp changed the title chore: add lint Makefile task chore: add lint and fmt Makefile tasks Aug 19, 2022
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
evan-forbes
evan-forbes previously approved these changes Aug 19, 2022
@rootulp rootulp merged commit 32596b8 into celestiaorg:main Aug 22, 2022
@rootulp rootulp deleted the rp/makefile-lint branch August 22, 2022 14:02
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.

None yet

3 participants