[llvm-dev] [RFC] Helping release management

Tue May 17 13:54:51 PDT 2016

Hi Renato,

> On May 17, 2016, at 10:14 AM, Renato Golin <renato.golin at linaro.org> wrote:
> 
> On 17 May 2016 at 17:25, Quentin Colombet <qcolombet at apple.com> wrote:
>> I do not see how the proposal goes against that policy. I certainly believe that the discussion should stay in the bugzilla, but I do not see why this is incompatible with the fact of summarizing some information in the commit message.
>> Basically what I tend to push for is self contained information for quick reference in the log. Then, if people want more information, they are encouraged to check the bugzilla.
> 
> That is already clear on the current policy, though...
> 
> "The body should be concise, but explanatory, including a complete reasoning.
> Unless it is required to understand the change, examples, code snippets and
> gory details should be left to bug comments, web review or the mailing list."
> 
> But I got it that you want "some" things more specific. I'm just
> sceptical as to how much that will benefit the overall scheme of
> things.
> 
> I think this should be a proper review, separated from this thread,
> with the reasoning well explained. Let's focus on explaining it. :)

Fair enough.

> 
> 
>> Yes, links can rot, though http:/llvm.org/PR# is relatively stable (at least it did not break with the last migration), plus the PR number is still here.
>> In other words, I believe that link adds a convenient way to access bugzilla while still providing the default information if the link rots.
> 
> I'm not fussy about the URL, but changing the style now will mean all
> commits so far will behave differently, people will continue using the
> old style for a long time, and we'll have to support both ways
> forever.

I am fine keeping PR#, I just find it convenient for them to be clickable in mail.
Also, I do not see any problem supporting two styles. People have their preferences and the tools should adapt, not the other way around.

> 
> It should be hard to auto-link on any tool / web-page from the PR. I
> just don't see the benefit in this change.
> 
> Remember, this is about changing how people write commit messages and
> it's not a trivial matter. We already got used to doing this, unless
> there's a strong argument against the current model, I don't think we
> should change it.

What is missing form the suggested approach to work with the current policy?

As far as I can tell, what I explicitly called out is already interesting information that people more or less put.
The idea is to have a tool (hook) doing the boring things on top of our current practices. If this is not the case, then, yes, this is not going to work.

> 
> 
>> I miss the point.
>> For existing PR we would have: fixes PR1234.
>> For new PR we would have: fixes PRNew, where PRNew is a keyword that means create a PR and put the actual number in place of PRNew.
> 
> But that's not all there is to it...
> 
> The PRNew tag will mean: create a bug, add the text of this commit to
> it, mark it fixed by this commit, close it.
> 
> First, it'll require people actually remembering to put that, which in
> my view is as easy/hard to make them remember to open a bug.

If they want :).
I tend to believe that if that makes people's job easier (or equally painful) they would use it.

> 
> Second, what if this is a series of fixes, how to you connect this fix
> with others?

Good point. Thanks for bringing that up.
This use case actually goes beyond the conveniency I wanted to provide (i.e., not having to file a PR if this is a simple fix).
Indeed, if we need more than one commit to fix the problem I was thinking that we may want to create a PR first anyway.
Anyhow, if we decide to support that use case, we could choose to leave the PR open or have different keyword and stuff. Though, it seems to become complicated.
Like we discuss with Jim, I want the path of less resistance to do the right thing and adding a bunch of keywords seem against that.

> 
> Finally, and more importantly, who do we CC?

Why is this different from the current process of filing a PR or fixing a bug?

> To which component do we
> associate the bug?

We could use the default component or take advantage of the TAG in the title line.

> Which fix version?

I did not get that.

> Is this a candidate for
> back-port?

That should be answered by the answer of your next question.

> Is this a conformance or performance regression, or just a
> fix to a new bug?

This should be answered in the commit message already, i.e., people should document when the bug was introduced and what is fixed (performance, correctness, etc.)

I do not plan to make a commit hook smart enough to give/extract that information.

The commit hook is about documenting that a commit is a fix and the commit message is about answering all the interesting questions. But I do not plan to enforce any rules on them. People will need to use their own judgment to know what is best for the project.
If that means reminding contributors on their commit message what is helpful to release managers when fixing a bug, I do not see the harm.

> 
> All those questions require answers, and they either go blank (thus
> making the bug practically useless),

Yes, those are required answers, but I disagree that marking the bug is useless.
Marking that a commit is a fix is already an improvement compared to the current situation where it takes time to figure out if something is a fix, a new feature, a refactoring. Then, we can work with the contributor to get more information about the commit if that is missing.

> or they require further
> free-text-based encoding on what they are in the commit message.
> 
> I have had the unfortunate chance to work on free-text encodings [1]
> where some of the encoded part was well defined, others not so much.
> They were all equally horrid to work with. I'm really glad I don't
> ever have to look back.
> 
> Moreover, the experience was equally appalling to the users of our
> tools (people manually encoding the knowledge), so I can confidently
> say that using such a scheme, on any depth, will bring us misery.

Again, if the commit hook does not work on top of our current practice this is bad, and I do not want it.

> 
> 
>> Well, what I am trying to do is to make that automatic. I would be the first one annoyed if I have to file a PR before fixing any thing. If when I am fixing something, I just have to add “fixes PRNew”, that sounds easy enough to me :).
> 
> It would be as easy to write a script to automatically create a bug
> [2] for you from your current git branch, update your local PRNew
> tags, then commit.

Indeed.

> 
> The difference being that this will not get into the commit message
> policy, which was already regarded as too long and prone to be ignored
> by most people that contributed to that discussion…
> 
> If everyone uses your tool, great!

Well, if I need to change my workflow (e.g., using a different tool than git for the commit) for llvm compared to the other project I work on, that path is already dead for me. If we do not achieve transparency, we shouldn’t work into it.

> We still don't need to add to the
> commit policy.

Sorry for nitpicking your comment, but this is *not* a policy, this is a recommendation. We will not revert commit or whatever because of that. I am trying to instil what information is required by release managers to help them do their job. Contributors already know that information and most likely already put it in the commit message.

> If no one end up using it, equally great, we didn't
> change the policy for nothing.

I do not see the suggested patch as a change but as something explicitly stating what makes a commit message useful. If you disagree, that is fine you do not have to follow those suggestions. If nobody wants them to appear in our commit policy, that is fine as well. 

> 
> My whole point is, KISS. It's a lot easier to add things to a policy
> than it is to remove from them. I'd rather err on the side of caution.

Sounds fair, so I do not think this is actually changing anything, but I may miss something.

> 
> 
>> Sure, but I don’t see why you are opposing this two approaches: file a PR followed by a fix and file a PR with a fix.
>> Of course, people that don’t use the marking keywords will be out of the system, that I agree, but it seems to me that having automatic process like this will lower the bar for most people (assuming this is possible).
> 
> People out of the system will never care. People in the system that
> get things slightly wrong will be very annoyed.

Why?

> 
> Unless we're willing to reject any commit message that is malformed
> (and you can't tell if it's just malformed or ignored, so you have to
> reject *all* non-conforming), people with good intention will be the
> ones hurt the most.

> 
> I don't think we want to start rejecting commits because of the
> messages... or we'd have done by now.

Same here. I am trying to bring awareness of this problem. Even if this thread ends up without any action taken, I believe some people would have gain that awareness and that is already good (I am kind of an optimistic :)).

Cheers,
-Quentin

> 
> cheers,
> --renato
> 
> [1] http://arep.med.harvard.edu/labgc/jong/Fetch/SwissProtAll.html
> [2] https://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService/Bug.html