Add repo files to help maintain conventions #715
Conversation
- `.editorconfig` helps maintain whitespace rules and other things. For now, I'm just setting indenting and trailing whitespace rules. - `.gitattributes` sets checked in line ending rules. Without this file. each git client follows the rules configured by the machine that installed it. This can lead to large walls of diffs in PRs where nothing but the line endings changed. But with this file, we can constrain git in all installations to follow the line ending policies of this repo. Those policies, as I've been able to detect, are to check in LF line endings exclusively. This file ensures that policy continues to be consistently respected.
.gitattributes
Outdated
| ############################################################################### | ||
| # Set default behavior to automatically normalize line endings. | ||
| ############################################################################### | ||
| * text=auto | ||
|
|
There was a problem hiding this comment.
I don't agree we should change the default. For reference, what this does is convert files that git detects as text to have CRLF line endings when checked out on Windows. But I've never seen a programming-oriented text editor on Windows in the past two decades that didn't work just fine with LF-only files, preserving their LF-onlyness. The auto-conversion behaviour, on the other hand, is liable to cause things to break in unpredictable ways when building or testing from a checkout that has been converted.
If someone wants the conversion behaviour, they can and should set the core.autocrlf config variable to auto. I have no objection to the *.sh eol=lf line below that ensures that is all they have to do, i.e. that doing so does not break shell scripts.
| ############################################################################### | |
| # Set default behavior to automatically normalize line endings. | |
| ############################################################################### | |
| * text=auto |
From the PR description:
Without this file, each git client follows the rules configured by the machine that installed it. This can lead to large walls of diffs in PRs where nothing but the line endings changed.
Not in practice; I don't remember us ever receiving such a PR, and if we did we'd just tell the PR author that they should not be using any editor that does that.
There was a problem hiding this comment.
I don't agree we should change the default.
The default is that the client installation sets the policy. If you care about line endings being consistent for your repo (and it sounds like you do), you should establish a policy via .gitattributes.
If you disagree with this particular policy, that's totally fair and I'm happy to change it.
what this does is convert files that git detects as text to have CRLF line endings when checked out on Windows. But I've never seen a programming-oriented text editor on Windows in the past two decades that didn't work just fine with LF-only files, preserving their LF-onlyness. The auto-conversion behaviour, on the other hand, is liable to cause things to break in unpredictable ways when building or testing from a checkout that has been converted.
I've never seen that happen, but it sounds like you'd prefer LF to be checked in and LF to be checked out. That's a fine policy and I'll figure out what change to make to this file to encode that as a rule.
Without this file, each git client follows the rules configured by the machine that installed it. This can lead to large walls of diffs in PRs where nothing but the line endings changed.
Not in practice; I don't remember us ever receiving such a PR
The default behavior of a Git for Windows install is to check out CRLF line endings and check in LF. But as some folks feel that version control shouldn't ever mess with files' line endings, if they were to configure their client this way, and created a new text file on Windows, they'd introduce a CRLF line ending file to your repo and you'd probably never notice.
To be fair, you probably don't get that many PRs in a repo as highly specialized as this. And most of your contributors probably don't use Windows machines (which are more likely to try to rewrite things as CRLF. So while I accept that you haven't seen the PR that I describe so far, you can still save yourself and others the trouble of ever encountering that problem by encoding your line ending policy in your repo.
There was a problem hiding this comment.
Case in point, I just cloned this repo on linux WSL and the script failed because of a line ending issue:
andrew@ryzen9:~/git/zips$ ./render-via-docker.sh
-bash: ./render-via-docker.sh: /bin/bash^M: bad interpreter: No such file or directory
Once this .gitattributes file is checked in, I expect this to never happen again.
There was a problem hiding this comment.
The default is that the client installation sets the policy.
No, the default is to leave line endings alone. The client installation can override that, but this is by far not the only client git configuration override that can end up breaking things.
Case in point, I just cloned this repo on linux WSL and the script failed because of a line ending issue: [...]
It must have failed because you overrode the default policy in your user settings. (Note that "leave line endings alone" is also the default on Windows, for good reason.)
That will break scripts in checked-out git repos on WSL in general, because WSL is Linux (just running in a VM), and it uses the Unix line-ending convention. There's nothing special about this repo that should require it to be defensive about user settings that are poorly chosen for the environment they're running in, when the vast majority of git repos aren't defensive in that way.
In any case:
I have no objection to the
*.sh eol=lfline below that ensures that [overriding thetextsetting totext=auto] does not break shell scripts.
There was a problem hiding this comment.
The default is that the client installation sets the policy.
No, the default is to leave line endings alone. The client installation can override that, but this is by far not the only client git configuration override that can end up breaking things.
I think we may be arguing semantics here. You're talking about the default in a client installation. I'm talking about the default when the repo doesn't specify the policy.
The point is that users and installations have varying settings configured, and it can cause this repo to misbehave for them and possibly to get commits sent via PRs that have unwanted line endings. Setting the policy in code mitigates both of these problems.
It must have failed because you overrode the default policy in your user settings.
Quite possibly, but I don't remember doing so.
There's nothing special about this repo that should require it to be defensive about user settings that are poorly chosen for the environment they're running in
"Poorly chosen" is a subjective opinion. People choose what makes sense for them.
when the vast majority of git repos aren't defensive in that way.
I can't comment on the statistics in general. All I know is that all my repos are explicit about the line ending policy and it has saved myself and others a lot of hassle. It seems very safe to declare policies that you do have for this repo in the repo itself so that people and tooling knows what policies you want. I don't understand your resistance to codifying your policies, as it isn't as "inclusive" to hide policies so that only people in-the-know are aware of them.
.gitattributes
Outdated
|
|
||
| # Ensure shell scripts use LF line endings (linux only accepts LF) | ||
| *.sh eol=lf | ||
| *.ps1 eol=lf |
There was a problem hiding this comment.
This is unnecessary:
- we don't use any PowerShell scripts;
- PowerShell doesn't care about line endings.
| *.ps1 eol=lf |
There was a problem hiding this comment.
I'll remove it on the basis that you have no .ps1 scripts.
But FYI, while powershell doesn't care about line endings, linux does. And if you start your ps1 script with #!/env/pwsh or something similar, that file had better use LF line endings or linux will be unable to find pwsh due to the extra CR at the end of that line. That's why I added this line to all my .gitattributes files.
.editorconfig
Outdated
| ############################################################################### | ||
| # Set default behavior to automatically normalize line endings. | ||
| ############################################################################### | ||
| * text eol=lf |
There was a problem hiding this comment.
| * text eol=lf | |
| * -text |
There was a problem hiding this comment.
This policy states that you don't mind a mix of line endings. Within a file, line endings could be mixed (and you can blame the editor if you want), but certainly newly introduced files may be created with CRLF line endings. This policy that you propose should mean you're OK with that.
If you want to see LF line endings instead, we should state that.
| @@ -0,0 +1,7 @@ | |||
| ############################################################################### | |||
| # Set default behavior to automatically normalize line endings. | |||
There was a problem hiding this comment.
| # Set default behavior to automatically normalize line endings. | |
| # Set default behaviour to not attempt to normalize line endings. |
There was a problem hiding this comment.
behavior is the US English spelling of the word. Is there a policy to use British English in the repo?
.editorconfighelps maintain whitespace rules and other things. For now, I'm just setting indenting and trailing whitespace rules..gitattributessets checked in line ending rules. Without this file. each git client follows the rules configured by the machine that installed it. This can lead to large walls of diffs in PRs where nothing but the line endings changed. But with this file, we can constrain git in all installations to follow the line ending policies of this repo. Those policies, as I've been able to detect, are to check in LF line endings exclusively. This file ensures that policy continues to be consistently respected.