Skip to content

Conversation

mgaffigan
Copy link
Contributor

@mgaffigan mgaffigan commented May 30, 2025

Add .gitattributes to ensure EOL is reasonable for new files

Re: #110. Should not be merged without git add --renormalize . commit. For an example commit, see https://github.com/OpenIntegrationEngine/engine/compare/main...mgaffigan:feature/add-gitattributes-renormalize?expand=1&w=1

Renormalize requirement avoided in e0989b4. See discussion in issue for cause.

@mgaffigan
Copy link
Contributor Author

Updated to avoid requirement for large commit, also added six files which have mixed end-of-line within the same file. Diff which ignores eol changes can be viewed at https://github.com/OpenIntegrationEngine/engine/pull/111/files?w=1

Copy link
Member

@tonygermano tonygermano left a comment

Choose a reason for hiding this comment

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

The mixed eol commits that rewrite a significant part of the file are what I was originally concerned about in my comments in #110 . I've since learned about .git-blame-ignore-revs, which supposedly is supported by github, so I'm planning to test that out.

Assuming it works, then my concern about the git blame will not be an issue when we fully normalize as you had suggested in the issue. The only issue at that point is that we'd be creating merge conflicts for any sharing between forks if the other forks do not normalize as well.

@jonbartels jonbartels requested review from a team, kayyagari, gibson9583, ssrowe, kpalang, tonygermano and pacmano1 and removed request for a team June 17, 2025 12:41
Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
@jonbartels jonbartels force-pushed the feature/add-gitattributes branch from 7293d9a to f20c4ed Compare June 17, 2025 12:45
@mgaffigan
Copy link
Contributor Author

@tonygermano, I did look at git-blame-ignore-revs, but since it only applies in a non-default configuration, did not include it.

If the issue is limited to the 6-files, it is still valuable to merge without that to prevent further issues in the future. The 6-file count is reduced from the original 600 that git add --renormalize . wanted. If there's consensus on the 6-whitespace-only edits being too much, then I'm happy to revert them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants