-
Notifications
You must be signed in to change notification settings - Fork 837
remove todo, add gitSHA into version information #227
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
Conversation
gaocegege
left a comment
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.
LGTM
Makefile
Outdated
| @@ -0,0 +1,28 @@ | |||
| # Current version of the project. | |||
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.
This Makefile isn't invoked by our CI/CD test infrastructure so the changes wouldn't actually be picked up by the released artifacts.
The current approach is to use Python scripts to invoke the build steps (bazel has been discussed as a possible solution).
This is the Python function that invokes go build in our continuous tests
https://github.com/tensorflow/k8s/blob/master/py/release.py#L138
Our continuous release process is just a cron job that invokes that script
https://github.com/tensorflow/k8s/blob/master/release/releaser.yaml
So I think you just want to update the go command line here
https://github.com/tensorflow/k8s/blob/master/py/release.py#L138
And then the changes would be picked up.
|
@jlewi thanks you very much, will do it |
|
@zjj2wry Let me know when this is ready for another look. |
|
@jlewi sorry for delay, this ready, ptal |
jlewi
left a comment
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!
This change is