-
Notifications
You must be signed in to change notification settings - Fork 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
Move resource version annotation to a different file #4018
Conversation
3295ee5
to
609bfb3
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.
One small change but otherwise LGTM.
The commit hash will change every run, so you probably need to tweak the test code to strip those out before doing deltas. Timestamp too. I had to do that with the prior version. You may also want to not store all the annotations (in the actual code, not the test code), but I didn't mention it before because it's harmless that they are there; they are ignored by your |
Additional things I can find are creationTimestamp & uid. Instead of removing specific annotations, I hope we can dump them only. As we have made it a hidden file it should not affect the user too. |
609bfb3
to
f4f4c31
Compare
f4f4c31
to
793a874
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.
A small comment, but otherwise looks good.
commands/alpha/rpkg/pull/command.go
Outdated
@@ -113,7 +113,7 @@ func (r *runner) runE(_ *cobra.Command, args []string) error { | |||
return errors.E(op, err) | |||
} | |||
|
|||
if err := util.AddResourceVersionAnnotation(&resources); err != nil { | |||
if err := util.AddRevisionMetaData(&resources); err != nil { |
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.
Nit: We should use consistent naming, so this should probably be AddRevisionMetadata
.
793a874
to
5d2b47e
Compare
The newly added file can be used for storing all the metadata going forward It is added and removed from the client side and will not be present in porch Please refer this for more info: kptdev#4017
5d2b47e
to
faaf95b
Compare
The newly added file can be used for storing all the metadata going forward It is added and removed from the client side and will not be present in porch Please refer this for more info: kptdev#4017
Moving resource version annotation to a new file. The newly added file can be used for storing all the metadata going forward. It is added and removed from the client side and will not be present in porch.
Please refer this for more info: #4017