-
Notifications
You must be signed in to change notification settings - Fork 8
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
Align Makefile - Install Tools #3
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.
one nit, otherwise lgtm
Makefile
Outdated
$(call operator-framework-tool, $(OPERATOR_SDK), $(OPERATOR_SDK_DIR),github.com/operator-framework/operator-sdk/releases/download/$(OPERATOR_SDK_VERSION)/operator-sdk_$${OS}_$${ARCH}) | ||
|
||
# operator-framework-tool will delete old package $2, then dowmload $3 to $1. | ||
define operator-framework-tool |
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.
Can we have a better name for this please? operator-framework-tool does not make sense to me
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.
How about install-operator-framework-tool
or operator-framework-install-tool
?
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.
my 2:coin:
Since this uses curl
, and so it's generic (respect go-install-tool
), why not just install-tool
?
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.
Since this uses curl, and so it's generic (respect go-install-tool),
I agree that it not specific and not related to operator-framework
...
why not just install-tool ?
Good option but I lean to url-install-tool
since it install the tool based on it's URL
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.
Good option but I lean to url-install-tool since it install the tool based on it's URL
Which is better indeed 👍
In the process of moving FAR to Medik8s organization we align the Makefile - third PR.
bundle
directorybuild-tools
target to verify that all tools have been updated locally.