Skip to content

Make sure to update all golden files when running make gen#20873

Merged
istio-testing merged 1 commit intoistio:masterfrom
dgn:dgn/improve-gen-target
Feb 5, 2020
Merged

Make sure to update all golden files when running make gen#20873
istio-testing merged 1 commit intoistio:masterfrom
dgn:dgn/improve-gen-target

Conversation

@dgn
Copy link
Copy Markdown
Contributor

@dgn dgn commented Feb 5, 2020

We didn't update the pkg/kube/inject golden files on make gen, which is why make gen-check doesn't catch it when you forget to update them.

@dgn dgn requested review from a team, costinm, howardjohn, linsun and rshriram as code owners February 5, 2020 11:09
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 5, 2020
@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 5, 2020
@howardjohn
Copy link
Copy Markdown
Member

but the unit tests fail if these are outdated? it seems a bit weird to have this in gen?

@dgn
Copy link
Copy Markdown
Contributor Author

dgn commented Feb 5, 2020

but the unit tests fail if these are outdated? it seems a bit weird to have this in gen?

Why? We're also re-generating the operator golden files in gen. I think we should have a single target to update golden files used in unit tests. That way I know that when I have a failure because of a golden file, I can just run that target and be done with it, and don't have to look through the code to see if I have to set REFRESH_GOLDEN or UPDATE_GOLDEN to true, or whatever the next person comes up with

Copy link
Copy Markdown
Member

@howardjohn howardjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I think that makes sense

@istio-testing istio-testing merged commit b39d1ac into istio:master Feb 5, 2020
@dgn dgn deleted the dgn/improve-gen-target branch February 13, 2020 09:19
sdake pushed a commit to sdake/istio that referenced this pull request Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants