-
Notifications
You must be signed in to change notification settings - Fork 168
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
Improve update-codegen.sh to copy generated files into SEDNA_ROOT. #218
Improve update-codegen.sh to copy generated files into SEDNA_ROOT. #218
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.
Thank you for pointing it out.
hack/update-codegen.sh
Outdated
@@ -26,3 +26,15 @@ ${SEDNA_ROOT}/hack/generate-groups.sh "deepcopy,client,informer,lister" \ | |||
${SEDNA_GO_PACKAGE}/pkg/client ${SEDNA_GO_PACKAGE}/pkg/apis \ | |||
"sedna:v1alpha1" \ | |||
--go-header-file ${SEDNA_ROOT}/hack/boilerplate/boilerplate.generatego.txt | |||
|
|||
# Check if Sedna home is different from the standard directory where GO projects are located | |||
if [ "${GOPATH}/src/${SEDNA_GO_PACKAGE}/" != "$${SEDNA_ROOT}/" ]; then |
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.
- $${SEDNA_ROOT} => ${SEDNA_ROOT}
- we need to check they are the same location instead of only checking equal string, such as symbol link relation
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.
More word, the user may create the symbol link directory for
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.
Makes sense. Did you have in mind something like this:
if ! [[ ${GOPATH}/src/${SEDNA_GO_PACKAGE}/ -ef ${SEDNA_ROOT}/ ]]; then ...
This should also help with symbolic links comparison.
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.
-ef
is good choice.
PS: quote "${SEDNA_ROOT}" is a good habit
d804a4c
to
86e32f2
Compare
Signed-off-by: Vittorio Cozzolino <[email protected]>
87774b5
to
ea2f333
Compare
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: llhuii 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 |
kubernetes/gengo#215 would add module support for kubernetes gengo. After that effort has merged, we don't need this rsync avoidance. |
For |
The
update-codegen.sh
script won't work if the user cloned Sedna into a custom folder that is different from the standard one for GO projects. For someone new to GO, this can be very misleading as no errors are prompted and the user will never see the updated generated code. I think that this is a high-priority change as I can see many people having this problem, myself included.This patch enhances the script so that it will copy the generated code into the
SEDNA_ROOT
, if necessary.