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
gerrit: complains about commit bodies wrapped at 72 chars #17941
Comments
The choice may be subjective, but the default |
For comparison, vim seems to default to 72:
I don't think editor defaults should play a major role here, though. |
Just ignore the warning. I do. No person should care how many characters are on a line, but for a computer to care is just ridiculous. |
@robpike given that git doesn't play well with non-wrapped commit bodies, I do think a warning (not an error) like this is somewhat useful for people who aren't wrapping their non-quoted text. What I'm proposing is that since 72 chars seems to be common practice and still plays well with git, the warning should be made less strict. Removing the warning altogether is also an option, but this change is a simple compromise. |
I'm not sure this is even a Gerrit setting. @spearce? Quick Google search didn't find anything at least. I didn't try too hard. |
@bradfitz have you looked at https://bugs.chromium.org/p/gerrit/issues/detail?id=841? It's something that gerrit remote is saying, so I don't know where else this could be set. It's definitely not a git thing. |
Ah, great. I missed that. For reference,
|
Okay, I sent https://go-review.googlesource.com/#/c/33323/ to @ianlancetaylor adding:
... to the Go All-Projects configuration. |
Thanks! Any rationale behind the subject length being so large? Usually it's kept lower, like 50 as mentioned in the links I provided or 65 according to gerrit. |
Is there some reason we're picking 76? Is it some celebration of the signing of the US Declaration of Independence? Can't it be something useful, like 100 or 200 or 1e6? I need to oil my 026 keypunch http://www.columbia.edu/cu/computinghistory/026.html |
I'm bored of this bug. I'll let somebody else take this over. |
A year later, I have taken this over, in part because the default limits seem to be decreasing over time. I set both limits to 1000 in https://go-review.googlesource.com/c/All-Projects/+/78090/3/project.config. I hope we can all agree that lines longer than 1000 characters do deserve a warning. |
OK, that CL did nothing. For future reference, the right magic appears to be:
or else to have edited the gerrit.config stored in Google's internal repos and waited for the next (day or two) config push. Update: Apologies for continuing to dump this here, but we might be able to find it later if I do. Apparently after running those commands the suggested next step is to update the internal devtools/gerritcodereview/hosted_sites/go/etc/gerrit.config, send that out as an internal CL, wait until it gets pushed, and then unset the gob-ctl commands. The gob-ctl commands are persisted but harder to see, so it's better have config in the file and use the gob-ctl commands as temporary band-aids over the main config file to reduce the latency of making changes. |
It is my understanding that this is a setting that can be changed in gerrit. Is there a reason why 70 was chosen?
As far as I can tell, 72 is the most commonly used wrapping length for commit bodies in git:
http://chris.beams.io/posts/git-commit/#wrap-72
https://bugs.chromium.org/p/gerrit/issues/detail?id=841
https://medium.com/@preslavrachev/what-s-with-the-50-72-rule-8a906f61f09c#.3154gadgk
At the same time, I can't find any reference to this in any official Git docs.
Is there a reason why our gerrit instance is configured like this?
The text was updated successfully, but these errors were encountered: