-
Notifications
You must be signed in to change notification settings - Fork 17
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
Adding a Makefile and bumping version of dependency #41
Conversation
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.
Thanks for the PR!
It LGTM, but could you please split the changes into separate commits? Maybe 4: Makefile, dep upgrade, .gitignore and README? So that we could easily roll something back if needed.
I really don't think the Makefile adds anything - most things in it replace I also in general dislike Makefiles in go projects ... because in most cases they add nothing 🤣 |
Yeah, I don't use them either in Go projects, since I often need to change commands slightly. I would be lost without command history and But if it helps someone, why not? 🤷♂️ |
Definitely doesn't add a lot, but does make it nice to As for the addition of a |
9110147
to
2731f46
Compare
I'll stay with the @javaducky, the E.g. today we need run tests using:
tomorrow it can be
The other thing is standardization, e.g. we have in may project standard make target #!/bin/bash
output=$(make -n check 2>&1 | head -1)
if [[ "$output" != *"No rule to make target"* ]]; then
echo "Running check"
make check
fi which shift left the issue detecting and saves me time |
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.
Thanks Paul!
Very minor stuff while I was looking around. Figured I'd add a Makefile to make building locally a bit easier.
Don't know enough on the CI setup just yet, so I didn't change the Go version in
go.mod
.