-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
doc: slightly relax 50 character rule #16523
Conversation
Allow commit message first line to exceed 50 chars if necessary
+1: it's usually tricky to fit in 50 characters when the subsystem is child_process |
would we rather do this vs switch to something like 72 columns for the title? To me, the fewer exceptions we make, the easier it is to automate around it |
@evanlucas I tried that back in #8327 but maybe people have changed their mind since :) I’d really like that. |
I'd be happy with increasing to 72 characters but perhaps lets try this first and see how it goes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No strong opinion on this, but I would prefer to relax the 50 char limit instead of making 72 the new "hard limit", mainly because this is a Git convention (as far as I know). And I am pretty sure GitHub will cut long titles at a certain length, even though I don't know whether that's less than 72 chars.
/CC @nodejs/automation |
Can we make sure we stick within the limit of what github will take without wrapping it into the description? |
That’s 72, so: Yes, we’re going to do that anyway. |
PR-URL: #16454 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
CONTRIBUTING.md
Outdated
- be 50 characters or less | ||
- be 50 characters or less (or as close as possible to 50 characters if it | ||
is necessary to go over in order to provide a *useful* description of the | ||
change) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: how about remove this entire bullet point and change the first bullet point to this:
- contain a short description of the change (preferably 50 characters or less)
All, please take a look, fairly significant change. |
Still LGTM. |
CONTRIBUTING.md
Outdated
- contain a short description of the change | ||
- be 50 characters or less | ||
- contain a short description of the change (preferably 50 characters or less, | ||
and absolutely no more than 72 characters) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Micro-nit, non-blocking obviously: remove absolutely
Allow commit message first line to exceed 50 chars if necessary PR-URL: #16523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Landed in 1cdcab0 |
Allow commit message first line to exceed 50 chars if necessary PR-URL: #16523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Allow commit message first line to exceed 50 chars if necessary PR-URL: #16523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Allow commit message first line to exceed 50 chars if necessary PR-URL: #16523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Allow commit message first line to exceed 50 chars if necessary PR-URL: nodejs/node#16523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Allow commit message first line to exceed 50 chars if necessary PR-URL: nodejs/node#16523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Allow commit message first line to exceed 50 chars if necessary PR-URL: #16523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Allow commit message first line to exceed 50 chars if necessary PR-URL: #16523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Allow commit message first line to exceed 50 chars if necessary PR-URL: #16523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Allow commit message first line to exceed 50 chars if necessary PR-URL: nodejs/node#16523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
not important, and i know this is old news; just wondering, but why 72 instead of the standard 80? |
@Announcement Let's see what Linus Torvalds, the inventor of Git, has to say about that:
|
Allow commit message first line to exceed 50 chars if necessary
ping @Trott ... btw, note that the contribution guide already uses "should* and not "must"
Checklist
Affected core subsystem(s)
doc