Bug 27754 – commit-log-editor does not produce a git commit log that is git friendly.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Pierre d'Herbemont]

commit-log-editor does not produce a git commit that is git friendly.

git will always use the first line of the commit as a convenient short log. Having the date and the committer at this place is not convenient. Moreover we don't need the ident.

A more suitable format would be:

"
<short bug description>
<bug link>

<reviewed by >

<More description>

<Function description>
"

[Pierre d'Herbemont]

Created attachment 33598 [details]
commit-log-editor does not produce a git commit log that is git friendly.

https://bugs.webkit.org/show_bug.cgi?id=27754

Reviewed by NOBODY (OOPS!).

We make sure we end up with:
- A first paragraph describing the bug.
- The Reviewed By line
- The rest of the commit

We get rid of the date given that this is part of the commit info
in a more precise maneer, and also the author of the patch,
as this is also part of the commit information. Given that subversion
doesn't support different committer and author info, the line
would have to be added manually.

* Scripts/commit-log-editor:
---
 2 files changed, 51 insertions(+), 1 deletions(-)

[Pierre d'Herbemont]

Created attachment 33601 [details]
commit-log-editor does not produce a git commit log that is git friendly.

https://bugs.webkit.org/show_bug.cgi?id=27754

Reviewed by NOBODY (OOPS!).

We make sure we end up with:
- A first paragraph describing the bug.
- The Reviewed By line.
- An eventual Patch By line if author and committer doesn't match.
- The rest of the commit.

* Scripts/commit-log-editor:
---
 2 files changed, 66 insertions(+), 2 deletions(-)

[Pierre d'Herbemont]

Created attachment 33602 [details]
commit-log-editor does not produce a git commit log that is git friendly.

https://bugs.webkit.org/show_bug.cgi?id=27754

Reviewed by NOBODY (OOPS!).

We make sure we end up with:
- A first paragraph describing the bug. It is eventually prefixed by
"WebKit: <line>" or "WebCore: <line>". This used to be
"WebCore:\n\n<line>".
- The Reviewed By line.
- An eventual Patch By line if author and committer doesn't match.
- The rest of the commit.

* Scripts/commit-log-editor:
---
 2 files changed, 69 insertions(+), 3 deletions(-)

[Eric Seidel (no email)]

I'm confused.  Don't we need "svn friendly" commit logs anyway?  The git commit log is turned into the svn commit log during git svn dcommit...

[Pierre d'Herbemont]

(In reply to comment #4)
> I'm confused.  Don't we need "svn friendly" commit logs anyway?  The git commit
> log is turned into the svn commit log during git svn dcommit...

Right. This will alter the way commit log are in svn commit log message as well. I have seen some commit log formatted that way [1], and I liked it. I think it's "svn friendly" as well.

We could probably change the name of the bug to reflect that.

[1] http://trac.webkit.org/changeset/46449

[Adam Treat]

I'm only Ok with this if it is the same for svn.  If we're going to change the commit log messages then we should do the same for whatever scm we're using. 

That said, even were we to do this for svn too, I'm not sure I like it as the whole idea is for the commit messages to exactly reflect the ChangeLog format.

Rather, I'd like to revisit this after Maciej's patch process plan is completed and we can look into changing to another scm from svn (aka Git)  I think this discussion would be moot given such an eventuality as git would allow us to get rid of ChangeLog's altogether.  We could then revisit the format...

[Pierre d'Herbemont]

(In reply to comment #6)
> I'm only Ok with this if it is the same for svn.  If we're going to change the
> commit log messages then we should do the same for whatever scm we're using. 

It's actually independent from the SCM.

> That said, even were we to do this for svn too, I'm not sure I like it as the
> whole idea is for the commit messages to exactly reflect the ChangeLog format.

The problem with ChangeLog format is indentation and the first line, which is a duplication of what is already in the commit meta data. Where it has to retain the same info, its presentation has to be different.

How ChangeLog are currently formatted is not well suited for commit log. I believe this is because the first approximation it to simply copy the ChangeLog content. I don't think this approximation is enough.

> Rather, I'd like to revisit this after Maciej's patch process plan is completed
> and we can look into changing to another scm from svn (aka Git)  I think this
> discussion would be moot given such an eventuality as git would allow us to get
> rid of ChangeLog's altogether.  We could then revisit the format...

I am afraid that such a plan is bound to take much more time than what is expected :)

But that decision seems to be wise though.

Pierre.

[David Kilzer (:ddkilzer)]

I do something similar now by hand where I extract common lines from the top of each ChangeLog and put them above each subsection:  <http://trac.webkit.org/changeset/45750>

I think we should change prepare-ChangeLog to add a line like this before the "Reviewed by" line:

        <http://webkit.org/b/00000> Enter one-line bug description here

        Reviewed by NOBODY(OOPS!).

Then these two common lines could be extracted from each ChangeLog entry when the commit message is generated.  (I was also thinking that the "00000" could be a place-holder that bugzilla-tool replaces when it lands bugs like it does for the reviewer.)

[Eric Seidel (no email)]

Comment on attachment 33602 [details]
commit-log-editor does not produce a git commit log that is git friendly.

So sad!  We have some parsing code like this in our python modules... Seems we really should just re-write commit-log-editor in python (at least then we could unit test it!)

Seems fine to me. :)  You might want to post to webkit-dev too so you can enjoy some quality bike-shedding...

Glad to see more folks working on our tools.  I would strongly encourage you to think about moving more of this code over to python in the future as our scripts seem to be moving that way. :)

[Adam Barth]

Pierre, are you a committer?  I don't see you listed at <https://trac.webkit.org/wiki/WebKit%20Team>.  In it's current state, no one will land this patch for you because it's assigned to you.  If you'd like it landed, you can set the commit-queue flag to ? and/or reset the assignee to the default using the "Assigned To" field.

[Pierre d'Herbemont]

(In reply to comment #10)
> Pierre, are you a committer?  I don't see you listed at
> <https://trac.webkit.org/wiki/WebKit%20Team>.  In it's current state, no one
> will land this patch for you because it's assigned to you.  If you'd like it
> landed, you can set the commit-queue flag to ? and/or reset the assignee to the
> default using the "Assigned To" field.

I should be able to commit. I'll add myself to that page, thanks Adam!

[Eric Seidel (no email)]

You'll also want to add yourself to WebKitTools/Scripts/modules/committers.py

I'll ask Adele about updating the committer invite letter.  Or maybe I'll just add a wiki page about being a new committer.

[Pierre d'Herbemont]

(In reply to comment #12)
> You'll also want to add yourself to WebKitTools/Scripts/modules/committers.py

For some reasons I can't commit anymore from here.

While this is sorted out I'll let someone apply the patch.

Pierre.

[Eric Seidel (no email)]

Comment on attachment 33602 [details]
commit-log-editor does not produce a git commit log that is git friendly.

If you're not in committers.py, your setting commit-queue will fail. :)

[Adam Barth]

(In reply to comment #14)
> (From update of attachment 33602 [details])
> If you're not in committers.py, your setting commit-queue will fail. :)

I just added him.  :)

[Adam Barth]

Comment on attachment 33602 [details]
commit-log-editor does not produce a git commit log that is git friendly.

Clearing review flag on attachment: 33602

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/commit-log-editor
Committed r46899
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/commit-log-editor
r46899 = 29faa92e5a1dd16dabeb13fb4acf53a3b1b8b2c0 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46899

[Adam Barth]

All reviewed patches have been landed.  Closing bug.