Skip to content

Conversation

@krtkvrm
Copy link
Contributor

@krtkvrm krtkvrm commented Aug 11, 2025

Summary

  • Add Makefile with common CI tasks (fmt, test, race, lint, ci)
  • Replace manual commands in GitHub workflows with make targets
  • Provides consistent development experience between local and CI

@krtkvrm
Copy link
Contributor Author

krtkvrm commented Aug 11, 2025

partially helping solve #281

@jba
Copy link
Contributor

jba commented Aug 11, 2025

I'm sorry, but this feels like overkill. You've got a complex makefile that people must understand and maintain, but there is no compelling reason for make—no real dependencies other than what the go command already handles.

If you want to add something to our workflow, add go vet and staticcheck to our lint step.

@krtkvrm
Copy link
Contributor Author

krtkvrm commented Aug 11, 2025

Thanks for the feedback! I understand the complexity concern. The Makefile is just shortcuts so contributors don't have to remember go test -v ./... every time or figure out the race detector command. Makes it easier to run the same stuff that CI does locally.

Will definitely add go vet and staticcheck to the lint step. Mind if golangci-lint gets added too? It catches a lot of issues that the others miss.

Also, would you be open to keeping just the basic targets like make test and make lint?

@jba
Copy link
Contributor

jba commented Aug 11, 2025

Sorry, maybe I wasn't clear. No makefile.

Also, no golangci-lint. It has too many false positives.

@wkoszek
Copy link
Contributor

wkoszek commented Aug 12, 2025

@jba having some ci.sh or a make shortcut would be good for contributors. Instead of wasting time at #168 because I forgot some space or a tab, it'd be nice to just run a simple script to build/test everything for me.

@krtkvrm
Copy link
Contributor Author

krtkvrm commented Aug 21, 2025

@jba closing this as I see staticcheck is added in CI

@krtkvrm krtkvrm closed this Aug 21, 2025
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.

3 participants