cluster-api icon indicating copy to clipboard operation
cluster-api copied to clipboard

🌱 Add .gitattributes file to hide generated diffs

Open killianmuldoon opened this issue 2 years ago • 7 comments

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.

killianmuldoon avatar Aug 10 '22 12:08 killianmuldoon

@killianmuldoon: This issue is currently awaiting triage.

If CAPI contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

k8s-ci-robot avatar Aug 10 '22 12:08 k8s-ci-robot

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.

killianmuldoon avatar Aug 11 '22 13:08 killianmuldoon

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. https://github.com/kubernetes-sigs/cluster-api/blob/05f10fdeb0130ccabec5d8799a194acd26078a4c/Makefile#L471-L474

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.

sbueringer avatar Aug 15 '22 16:08 sbueringer

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.

killianmuldoon avatar Aug 15 '22 17:08 killianmuldoon

Ah got it. Can you please add a comment to the .gitattributes file to explain that there as well? Thank you!

sbueringer avatar Aug 15 '22 17:08 sbueringer

/lgtm

sbueringer avatar Aug 16 '22 09:08 sbueringer

/approve

sbueringer avatar Aug 18 '22 10:08 sbueringer

[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 /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot avatar Aug 18 '22 10:08 k8s-ci-robot