-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
🌱 Add .gitattributes file to hide generated diffs #7045
🌱 Add .gitattributes file to hide generated diffs #7045
Conversation
@killianmuldoon: This issue is currently awaiting triage. If CAPI contributors determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
68d5757
to
f3fca14
Compare
9f78afc
to
5d1d8d0
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.
/lgtm
How about also adding the zz_generated
go files. Looks like github already does that using this check (thanks for finding this @killianmuldoon) but it might be good to make it explicit.
Could do, but I'd prefer to use the defaulting for now to see what the weaknesses are and where it might fail. Going forward though I'd expect to add a number of additional generated files to this. |
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.
Sounds good. 👍
I might be missing context, but why do we want to hide the diff? For me it sounds rather surprising. I also wonder if that means that our CI jobs won't fail now as an outdated generated YAML might not show up anymore in git diff? e.g. Lines 471 to 474 in 05f10fd
Or does it only hide it on the GitHub PR diff? But I also find it rather confusing if we can't see the diff there anymore. |
This only hides the diff by default on the PR view, pretty much just making it easier to scroll on that page - github already automatically does this with our generated go code and with "large" files. |
Ah got it. Can you please add a comment to the .gitattributes file to explain that there as well? Thank you! |
Signed-off-by: killianmuldoon <[email protected]>
5d1d8d0
to
7be7dc5
Compare
/lgtm |
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
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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 |
Signed-off-by: killianmuldoon [email protected]
Add a
.gitattributes
file to hide generated CRD yamls in PRs. Our generated go code is already automatically hidden by github linguist.The
.gitattributes
file lets us customize it's config - I'm sure there will be more potential additions to this in future.