Skip to content

Add gitattributes file - #190

Merged
jeremylong merged 1 commit into
package-url:masterfrom
dwalluck:gitattributes
Mar 16, 2025
Merged

Add gitattributes file#190
jeremylong merged 1 commit into
package-url:masterfrom
dwalluck:gitattributes

Conversation

@dwalluck

Copy link
Copy Markdown
Contributor

No description provided.

@dwalluckdwalluck mentioned this pull request Mar 12, 2025
Comment thread.gitattributes Outdated
@@ -0,0 +1 @@
* -text

@ppkarwaszppkarwaszMar 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
*-text
*text=autoeol=lf

Unless I am mistaken, this should force Git to convert file with CRLF endings to LF, whenever they are committed.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The -text means leave unchanged, and yours means convert to lf. But, why do you want to convert Windows .bat, to lf`, for example?

We could leave the formatter to check on the pull request and assume the committer knows best.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The project doesn't have any Windows files as far as I know, but you're saying if someone adds one they should also edit the .gitattributes file?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was more thinking about a Windows user that adds a Java file with CRLF and the CI "strangely" fails due to the Spotless check, but you are right, we should allow some CRLF files without modification of .gitattributes.

What about:

* -text
# Files checked by Spotless
*.java text eol=lf
pom.xml text eol=lf

and we add rules if we add Spotless formatters?

BTW: IIRC *.bat files should work with LF, cmd.exe ignores the CR unless I am mistaken. On the other hand shell files fail miserably if CRLF is used and the error message is quite confusing (due to the invisible character).

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Doesn't spotless also check some of *.json, *.sh, or *.yml?

I think it's going to get messy trying to match all files. We still have left even after all that:

Some dotfiles
LICENSE
README.md

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doesn't spotless also check some of *.json, *.sh, or *.yml?

Spotless can certainly do that too, although in #189 I only added a rule for Java files and the pom.xml.

I am not strongly opinionated on this matter. I think that auto-converting CRLF to LF in Java files could help some contributors, but I am fine in disabling auto-conversion altogether.

For me what matters the most is that builds on Linux can be reproduced on Windows and viceversa. As long as both systems check out identical files from Git, I am fine with either solution.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@ppkarwasz I covered all existing files in the latest commit.

@ppkarwaszppkarwasz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@jeremylongjeremylong left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@jeremylong
jeremylong merged commit 2f1ad84 into package-url:masterMar 16, 2025
@dwalluck
dwalluck deleted the gitattributes branch April 15, 2025 21:39
Sign up for freeto 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.

3 participants

@dwalluck@jeremylong@ppkarwasz