-
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
docs: Add instructions to update & verify scripts #255
docs: Add instructions to update & verify scripts #255
Conversation
hack/update-codegen.sh
Outdated
# | ||
# Now checkout the appropriate branch for the Kubernetes version that | ||
# you're building for. For example if you're building for Kubernetes | ||
# 1.10.0 you would do the following: |
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 you shorten this to "you would checkout release-1.10
"? I don't think we need to explain everything ;)
Also, the tag is wrong.
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.
Actually you know what, I was wrong on my second point. It's better to checkout the tag as you did, than the branch, as I did.
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.
Fixed.
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.
Ah didn't see the second comment. Fixing.
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.
Fixed now.
a051039
to
816bf5e
Compare
hack/update-codegen.sh
Outdated
# | ||
# Now checkout the appropriate branch for the Kubernetes version that | ||
# you're building for. For example if you're building for Kubernetes | ||
# 1.10.0 you would do checkout the kubernetes-1.10.0 branch. |
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.
Extra "do", and please wrap kubernetes-1.10.0
in backticks 👍
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.
Fixed the extra "do".
Regarding the backticks, this is a bash comment? Sounds odd to me to add backticks the same way we don't do that in Golang comments. Is there a conventional practice I don't know about?
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 do that in Go comments as well ;)
Not sure if there's an official agreement but I think I've seen others do it as well.
0c68438
to
dd5f11b
Compare
hack/update-codegen.sh
Outdated
# | ||
# Now checkout the appropriate branch for the Kubernetes version that | ||
# you're building for. For example if you're building for Kubernetes | ||
# 1.10.0 you would checkout the `kubernetes-1.10.0 branch`. |
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.
based on using this just now to fix my local setup, the branch name was release-x.x.x
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 @elliott-davis. I've pushed a fix.
dd5f11b
to
3adc004
Compare
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.
LFAD.
The issues raised in the review are fixed.
No description provided.