-
Notifications
You must be signed in to change notification settings - Fork 20
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
use bingo to manage binary dependencies #58
base: main
Are you sure you want to change the base?
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.
Looks good! Bingo seems to simplify/clean up the tooling quite a bit.
@@ -20,7 +21,8 @@ const ( | |||
) | |||
|
|||
// PrepareType represents a subtype for prepare messages. | |||
//go:generate stringer -type=PrepareType | |||
// | |||
//go:generate source ../../.bingo/variables.env && ${STRINGER} -type=PrepareType |
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.
I am not a huge fan of loading the environment variables like this inside the go generate. Is it possible to simply use the go path where all dependencies are installed?
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.
Kindly correct me if i am wrong here, as of now i don't see the String()
method being generated do we even need this go:generate
line?
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.
@jeroenrinzema Any thoughts?
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.
with bingo the catch i see for this is that the binaries are versioned, so you would have to refer it like say stringer-1.0.1
in the go files and each time you are upgrading the version you have to come and change here. That is the hazzle that source ../../.bingo/variables.env
avoids
# Tools | ||
$(BUILD_DIR): | ||
@mkdir -p $@ |
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.
Could you please update the Makefile to make sure that the bingo dependencies are available before running commands such as make generate
?
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.
As of now i don't see a generate
task in the Makefile
even in the main branch.
If i am getting it right, do you want the binary dependencies to be present before running any make command?
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.
The default behavior of bingo is "lazy installation" i.e. it is installed just before it is used, for example if you see the below screenshot i have accessed $(GOLANGCI_LINT)
in 2 places one in the dependency section and other in the task body. The dependency section one installs the golangci-lint if there are changes to the module file of golangci-lint
Discussion: #44