Omr: Commit title and message requirements

[mstoodle]

Questions were raised in #1500 around the "pedantic" rules applied for 50 characters and periods or not.

Let's move the discussion out of the PR and into this issue.

[mstoodle]

I don't feel strongly about the period, but it certainly burns a character for zero actual value.

(removed argument about merge commits because commit titles aren't copied into merge commit titles)

50 characters is a reasonable line in the sand (I believe it was originally arrived at by studying Linux kernel commit title data). I have felt occasionally challenged to fit my titles into 50 characters, but have yet to be stumped.

Contributors have complete freedom (and are, in fact, encouraged) to write all kinds of detail into the commit message. But I feel like contributors should also respect the wording in the title line so that I don't have to read the detail if I don't need to. I try to think hard about my titles so that people can make a quick decision whether to read my fancy prose or not.

Another way to put it: why spend so much time crafting and reviewing the code only to hide it behind (forgive a blunt adjective) sloppy and overly wordy history? We should be trying to make our history something we can be proud of, IMO.

That's just me, of course :) .

[mgaudet]

My take: We use 50, no periods as an aspirational goal, but if the commit message needs more, so be it. We should define a hard upper bound though, and use a tool like GitCop to enforce that; So while the aspiration may be 50/72, we use GitCop to enforce 72/100 or something.

[lmaisons]

With regards to the 50 character title rule, as someone who has been frequently frustrated by this restriction, I've always found the idea of using the historic average title length as the newly enforced maximum as statistically unsatisfying. In a contextual vacuum, I would have probably defaulted to the 95th percentile / 2 sigma. And would have required serious justification for anything less than the 68th percentile / 1 sigma. But, despite searching, I have never found anything more rigorous than '50 characters was average'.

Now, we don't operate in a contextual vacuum, and given that a 2-sigma title length looks to eyeball at around 80 characters, that isn't feasible if we seek to have zero lines larger than 72 characters. But 1 sigma looks to be between 60-70 characters, which, honestly sounds close enough to me to 72 to scrap the title length distinction and just require the same length for all lines that are not blank.

[mstoodle]

Wonderfully detailed argument. And it could be titled "Propose all commit message lines < 72 characters"

Huh, that's only 48 characters without even trying... :)

I am not seriously responding as an argument for or against, I just found it amusing and hope no one will be offended by a joke as interjection.

Thanks for contributing your proposal and argument, @lmaisons !

[mstoodle]

I am not sure about an "aspirational" limit because I'll be sorely tempted to call any title longer than that a failure (because you "aspired" to make it shorter, but didn't). I'll also feel silly calling any contributor on a "failed" commit message because, well, it's still shorter than the hard limit, so.....what's the big deal? I sure don't want to cast myself as GitCop's security guard.

So let's make a limit and stick to it (until someone says it's too short and then we'll discuss how much longer to make it...oh, wait...).

Given that, I would say there are these two proposals:
1) 72/100 (72 characters in title, 100 characters in commit lines)
2) 72/72 (i.e. all commit message lines are limited to 72 characters)

I'll give it another couple of days for the community to make other proposals / arguments, and will then ask the committers to vote ( @charliegracie @jduimovich @0xdaryl @vijaysun-omr @youngar ).

[0xdaryl]

A third proposal is to leave things the way they are:

1. 50/72 (50 characters in title, 72 characters in the body where reasonable)

[mstoodle]

Yes, good point :) . I had meant to say that, so thanks for making it explicit!

[charliegracie]

4) 50/72 (50 characters in title where reasonable, 72 characters in the body where reasonable).

This leaves it up to the committer reviewing the change to ask for modifications to the commit message similar to coding style, etc. Most IDEs do not enforce or even warn about commit message titles that are longer than 50 characters. I find it annoying to have to copy the title into an editor (since neither of the IDEs I use regularly provide this) that tells me the column my cursor is on so I can verify my commit message title is not too long. I also have to do the same thing to verify every PR I review and regularly have to ask contributors to limit the text. When contributors push a new commit with the title fixed it automatically kicks of more CI testing. The CI infrastructure currently being used by this project regularly has trouble keeping up with our demand on PRs and merges so these extra tests are a real burden.

[mstoodle]

I suggest we don't get too hung up on some of the current problems with CI or how we should test these limits once we've decided what they should be. Although the issue isn't open yet, I think we should also re-evaluate how we have been doing CI testing for PRs to alleviate the pain @charliegracie mentions. I don't think that pain needs to be connected to our commit message lengths :) .

Just to be clear, @charliegracie, the difference between #3 and #4 is the "where reasonable" flexibility added to the commit message length, yes?

[rwy0717]

Whichever way we go, I think it should be left up to committers to decide when style breakages are acceptable. Realistically, there are too many special cases and stylistic considerations to strictly enforce a hard requirement.

Some people are writing obviously bad commit messages. Maybe we should fight bad style with better onboarding?

A few ideas:

• Rewrite the contributing guide as a short tutorial
• Prominently mention CONTRIBUTING.md on the README.md.
• Move the commit guidelines to the front and center of CONTRIBUTING.md.
• Provide editor specific resources for writing better commit messages.

[charliegracie]

Correct @mstoodle that is the difference between 3 and 4.

[mstoodle]

@charliegracie @youngar @0xdaryl @vijaysun-omr @jduimovich
Please respond with your vote for:

1. 72/100 (72 characters in title, 100 characters in commit lines)
2. 72/72 (i.e. all commit message lines are limited to 72 characters)
3. 50/72 (50 characters in title, 72 characters in the body where reasonable)
4. 50/72 (50 characters in title where reasonable, 72 characters in the body where reasonable).

Note that a vote for #3 is the status quo (nothing changes).

I'll call it (option with most votes) when we've all responded or on Wednesday 23:59:59 EST, whichever comes first. Thanks!

[rwy0717]

I want to suggest another option:

1. 70/72 (70 in title where reasonable, 72 in the body where reasonable)

I think the 50 character title limit is causing some people pain, and that’s a pretty good reason to bump up the limit. Github will display up to 70 characters in the title, so we may as well use that.

I don't really see a call to increase the body width, so I'm suggesting we just leave it.

[mstoodle]

Last day to vote, @charliegracie @youngar @0xdaryl @vijaysun-omr @jduimovich

No one has voted yet, which (for the record) translates to #3 (status quo)

[charliegracie]

vote 5 +1

[youngar]

Voting for 5 :+1: (70/72 where reasonable)

[mstoodle]

I vote for 3, at the risk of being seen as a jerk :) . But I'm ok with 5 (that doesn't count as a vote :) ).

I will assume a non-vote is a "don't care" as opposed to supporting some particular option. So if you really support a particular option please vote @jduimovich @vijaysun-omr @0xdaryl

Sadly, @0xdaryl is away on vacation, so I don't know that he'll get a chance to vote.

[vijaysun-omr]

After reading through the discussion here, I agree with the viewpoint expressed by @rwy0717 the most.
I feel that committers should have some latitude to make exceptions "where reasonable". I personally do not feel it is a great use of a committer's time looking at the number of characters by copying/pasting into an editor as @charliegracie mentioned.

I vote for 5.

[mstoodle]

Ok, that's 3 votes for #5, so we can update the contributing guidelines for titles under 70 characters (where reasonable) and body lines under 72 characters (where reasonable). I'll make the update asaic unless someone else gets there before me :) . Once the guidelines are pdated, I'll close this issue.

Thanks for all the input and discussion, everyone.

[mstoodle]

Ok, it was drawn to my attention that we had some procedural problems in this vote, so I'm reopening just to clear things up. I would like to clean this one up so that future votes have a clean precedent rather than a bad example.

First of all, I changed the meaning of a non-vote mid vote, so that was not a good thing to do, especially given one committer was on vacation and may have seen (and relied upon) the first statement but not the second. Regardless: the vote should be made clear up front and results interpretation definitely should not change mid vote.

Next, I did not directly invite all our committers to participate in the vote; I missed @alin00 who is still listed as a committer although she has not been active for some time (much longer than the 6 month period mentioned in the Eclipse handbook). That's no excuse, though: she did not resign the role and we have not voted to remove her committer status, so I should have included her in the list. I'm very sorry for this unthinking omission @alin00 !

The voting period should have been longer. In particular, a longer voting period would have allowed another (known to be vacationing) committer to participate in the vote.

Lastly, Eclipse voting rules really seem to want us to vote on a particular proposition with agree (+1), disagree (-1) and abstain (0). Since we're an Eclipse foundation project, we should really follow their conventions.

My apologies to everyone for the poor quality example.

[mstoodle]

Given all that, I would like to treat the last "vote" as a poll to identify the real proposal on which the committers will now vote. Even though I have already updated the guidelines, let's pretend that I have not yet done that.

The proposal is to change the commit guidelines from the original:

• 50/72 (50 characters in title, 72 characters in the body where reasonable)

to the following:

• 70/72 (70 characters in title where reasonable, 72 characters in the body where reasonable)

Committers: if you support this change, please vote +1 or Agree. If you do not support the change, please vote -1 or Disagree. You are also welcome to explicitly or implicitly vote 0 or Abstain (so not voting is an Abstain). If you really need to, you can change your vote throughout the period; I will count only the last vote from each committer. I would like to encourage all the committers to participate in this vote. I will set the voting period as 1 week from today, which I think gives us all a chance to vote.

@alin00 @charliegracie @0xdaryl @youngar @jduimovich @vijaysun-omr please respond before Wednesday September 6 at 11:59:59 EST with your vote.

If, after the voting is complete, the sum of all votes is > 0 then the proposal will be accepted. Anyone can appropriately update (or not) the guidelines once the vote has completed, but I will be responsible for making sure the guidelines are updated in timely fashion.

[charliegracie]

+1

[youngar]

+1

[alin00]

+1

If you want to strongly encourage people to respect the limits, you could try tooling that truncates or auto-wraps the commit comments on checkin.

[0xdaryl]

+1

[mstoodle]

+1

[vijaysun-omr]

+1

[mstoodle]

Voting has completed, and we're at +6. I know that felt like rubber stamping the earlier decision, but I wanted to set the right example for votes in our community. Thanks to the committers for bearing with me.

Because the earlier vote had already been acted upon by yours truly, there is no further action required at this point, and I'm going to close this issue.