Skip to content

Doc: Development/FAQ: be more strict about line lengths+bug reports - #1119

Closed
jnpkrn wants to merge 1 commit into
ClusterLabs:masterfrom
jnpkrn:doc-development-faq
Closed

Doc: Development/FAQ: be more strict about line lengths+bug reports#1119
jnpkrn wants to merge 1 commit into
ClusterLabs:masterfrom
jnpkrn:doc-development-faq

Conversation

@jnpkrn

@jnpkrn jnpkrn commented Aug 16, 2016

Copy link
Copy Markdown
Contributor

Changes + justification:

  1. commit line lengths, lowering the limit from 80 to 70-75:
    • commit summary is actually actively encouraged to fit into
      the 50 characters limit if possible so there is still quite
      a space provided on top of that: GIT-COMMIT(1), GITTUTORIAL(7)
    • it makes sense for the rest to follow lenght limit as
      recommended in the Internet Message Form (RFC 2822, note that
      sending the patch to ML is offered as an alternative), which
      is 78, with line-wrap limit in MUAs traditionally preset at 72
      that also falls into the proposed limit of 70-75, and such line
      length is also automatically enforced with native "gitcommit"
      filetype plugin in vim (to state another source of de facto
      standard)
    • 70-75 range is not made up but adapted from Linux kernel guidelines:
      http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?h=v4.7&id=523d939ef98fd712632d93a5a2b588e477a7565e#n655
    • overall tone is slightly shifted from a recommendation to a rule
  2. bug report references are welcome, but only if they are publicly
    viewable
    • it's of no use for the wide audience otherwise, hence private bug
      mapping should be tracked privately by those who would make a use
      of that directly and not clutter public logs of a free software
      project
  3. one occurrence of "explanation" turned into "description" as the
    former was a bit overloaded in two various contexts

@jnpkrn

jnpkrn commented Aug 16, 2016

Copy link
Copy Markdown
Contributor Author

re 1.: have already mentioned that at
#964 (comment)

re 2.: one example is #1117

Changes + justification:

1. commit line lengths, lowering the limit from 80 to 70-75:
  - commit summary is actually actively encouraged to fit into
    the 50 characters limit if possible so there is still quite
    a space provided on top of that: GIT-COMMIT(1), GITTUTORIAL(7)
  - it makes sense for the rest to follow lenght limit as
    recommended in the Internet Message Form (RFC 2822, note that
    sending the patch to ML is offered as an alternative), which
    is 78, with line-wrap limit in MUAs traditionally preset at 72
    that also falls into the proposed limit of 70-75, and such line
    length is also automatically enforced with native "gitcommit"
    filetype plugin in vim (to state another source of de facto
    standard)
  - 70-75 range is not made up but adapted from Linux kernel guidelines:
    http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?h=v4.7&id=523d939ef98fd712632d93a5a2b588e477a7565e#n655
  - overall tone is slightly shifted from a recommendation to a rule

2. bug report references are welcome, but only if they are publicly
   viewable
  - it's of no use for the wide audience otherwise, hence private bug
    mapping should be tracked *privately* by those who would make a use
    of that directly and not clutter public logs of a free software
    project

3. one occurrence of "explanation" turned into "description" as the
   former was a bit overloaded in two various contexts
@jnpkrn
jnpkrn force-pushed the doc-development-faq branch from d8291b6 to 62ea0f0 Compare August 16, 2016 14:55
@jnpkrn

jnpkrn commented Aug 18, 2016

Copy link
Copy Markdown
Contributor Author

re 2.: to be fair, there are more camps with such a "subversive" activity, such as the one I count myself into, e.g.
6aae854
That's simply an act of impoliteness to the wider community, full stop.

@kgaillot

Copy link
Copy Markdown

For the summary line, I'd rather not get too complicated -- one line of under 80 characters is fine for usability. There's no need to restrict commit messages based on SMTP RFCs. And more generally, we do offer these as "guidelines" -- very few of these are hard rules.

For the BZs, it looks like we've only been reasonably consistent with listing CLBZs, so we can put that in the guidelines and leave the rest unsaid. I disagree that private bugs should never be listed -- it provides important context for why the change was made, even if the original bug is not viewable.

@jnpkrn

jnpkrn commented Aug 18, 2016

Copy link
Copy Markdown
Contributor Author

On 18/08/16 07:21 -0700, Ken Gaillot wrote:

I disagree that private bugs should never be listed -- it provides
important context for why the change was made, even if the original
bug is not viewable.

It provides important context to you if you can actually view that bug
report. Otherwise, it only makes you down if you are interested in
that particular change. In such case, the transparency lacks behind
miserably on otherwise free software project.

Jan (Poki)

@jnpkrn

jnpkrn commented Aug 18, 2016

Copy link
Copy Markdown
Contributor Author

On 18/08/16 07:21 -0700, Ken Gaillot wrote:

For the summary line, I'd rather not get too complicated -- one line
of under 80 characters is fine for usability. There's no need to
restrict commit messages based on SMTP RFCs. And more generally, we
do offer these as "guidelines" -- very few of these are hard rules.

Ok, what do you base 80 characters limit on, then?
It's really not the healthy limit and I at least provided rationale for
that.

Jan (Poki)

@jnpkrn

jnpkrn commented Aug 19, 2016

Copy link
Copy Markdown
Contributor Author

On 18/08/16 19:00 +0200, Jan Pokorný wrote:

On 18/08/16 07:21 -0700, Ken Gaillot wrote:

For the summary line, I'd rather not get too complicated -- one line
of under 80 characters is fine for usability. There's no need to
restrict commit messages based on SMTP RFCs. And more generally, we
do offer these as "guidelines" -- very few of these are hard rules.

Ok, what do you base 80 characters limit on, then?
It's really not the healthy limit and I at least provided rationale for
that.

Another supporting argument for what I proposed is that you'll get
such ugly formatted commit titles on GitHub otherwise (as a reminder
of this rule, I guess):

e65e0c5

Jan (Poki)

@kgaillot

Copy link
Copy Markdown

Ok, what do you base 80 characters limit on, then? It's really not the healthy limit and I at least provided rationale for that.

Start with: Q.: Do we need a limit on commit line length? A.: No, all modern software can handle lines of arbitrary length without difficulty.

That leads to: Q.: Then why do we want a limit? A.: Readability.

It's not for compatibility with MUA-submitted patches, which make up, what, maybe 0.1% of all commits?

Any limit is somewhat arbitrary. "80" originates from the fixed physical limit of punchcards and early CRT character displays, which obviously no longer matters, but over the years it has become a widely adopted consensus value for line length, used in a wide variety of contexts.

There is a decent argument to be made for something that displays in one line in github PR titles and git log (without arguments). But it's merely slightly more convenient, and it doesn't hurt anything at all for it to run longer.

Looking at it again, I think I'd rather just drop the mention of line length at all. It's too nitpicky when we want to encourage contributions, and people naturally pick something reasonable anyway. If a particular contribution is excessive, we can mention that in the review. I'll take care of deleting it ...

@kgaillot kgaillot closed this Aug 19, 2016
@kgaillot

Copy link
Copy Markdown

I plan on going with: "The +explanation+ briefly describes the change (the git project recommends the entire summary line stay under 50 characters, but more is fine if needed for clarity). "

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.

2 participants