-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Set the output base for fitask #11411
Conversation
Makefile
Outdated
@@ -152,7 +152,8 @@ ${KOPS}: | |||
codegen: | |||
go install k8s.io/kops/upup/tools/generators/... | |||
${GOPATH_1ST}/bin/fitask --input-dirs k8s.io/kops/upup/pkg/fi/... \ | |||
--go-header-file "hack/boilerplate/boilerplate.generatego.txt" | |||
--go-header-file "hack/boilerplate/boilerplate.generatego.txt" \ | |||
--output-base ${GOPATH_1ST}/src/k8s.io/kops/ |
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.
Maybe quote this one too?
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.
Considering that the fitask path is unquoted too, a better solution was to just escape the spaces at source.
Also, the quoting on the header file path was not very useful.
29d55c6
to
54b4a1a
Compare
/retest |
Makefile
Outdated
${GOPATH_1ST}/bin/fitask \ | ||
--input-dirs k8s.io/kops/upup/pkg/fi/... \ | ||
--go-header-file hack/boilerplate/boilerplate.generatego.txt \ | ||
--output-base ${GOPATH_1ST}/src/k8s.io/kops/ |
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.
We're trying to reduce our reliance on GOPATH
, could we use KOPS_ROOT
instead? It would probably be good to compile the fitask binary into _output/bin
to match the TOOLS_BIN
references in hack/
.
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 point. Done.
5ab5488
to
4eca458
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rifelpet The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/cc @olemarkus @rifelpet