llvm dev - Nov 2012 - [LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?

On Nov 15, 2012, at 2:28 PM, Chandler Carruth <chandlerc at google.com>
wrote:
>> [cfe-commits] r167788 - in /cfe/trunk: lib/Analysis/CFG.cpp
test/Analysis/dead-stores.cpp
>> 
>> with
>> 
>> [cfe-commits] r167788 - Fix bad CFG construction bug when handling C++
'try' statements.
>> 
>> The first gives me just enough information (in my abbreviated mesage
list window) to see that there was a change to Clang's Analysis library; if
I look at the whole subject, I can see what files were changed (or the first
few, at least). But the second actually tells me what changed, and whether or
not it affects me.
>> 
>> Having this form will also encourage people to put a useful summary in
the first line of their commit, which is useful for anyone viewing history
(especially git users, where the one-line summary is very common).
>> 
>> Is there a reason we currently prefer files to log messages?
> 
> I suspect not. I would much prefer your suggestion. If you can
> implement the fix to our scripts (and get Tanya or Chris or Anton to
> give you access), I would be thrilled.
I'm in favor of it.  Of course, the truly awesomest thing would be something
like:

    [cfe-commits] r167788 - [lib/Analysis] Fix bad CFG construction bug when
handling C++ 'try' statements.

Even if it required some heuristics and horrible hackery in the commit script,
it would be worth it.

-Chris

On Thu, Nov 15, 2012 at 5:47 PM, Chris Lattner <clattner at apple.com>
wrote:
>
> On Nov 15, 2012, at 2:28 PM, Chandler Carruth <chandlerc at
google.com> wrote:
>
>>> [cfe-commits] r167788 - in /cfe/trunk: lib/Analysis/CFG.cpp
test/Analysis/dead-stores.cpp
>>>
>>> with
>>>
>>> [cfe-commits] r167788 - Fix bad CFG construction bug when handling
C++ 'try' statements.
>>>
>>> The first gives me just enough information (in my abbreviated
mesage list window) to see that there was a change to Clang's Analysis
library; if I look at the whole subject, I can see what files were changed (or
the first few, at least). But the second actually tells me what changed, and
whether or not it affects me.
>>>
>>> Having this form will also encourage people to put a useful summary
in the first line of their commit, which is useful for anyone viewing history
(especially git users, where the one-line summary is very common).
>>>
>>> Is there a reason we currently prefer files to log messages?
>>
>> I suspect not. I would much prefer your suggestion. If you can
>> implement the fix to our scripts (and get Tanya or Chris or Anton to
>> give you access), I would be thrilled.
>
> I'm in favor of it.  Of course, the truly awesomest thing would be
something like:
>
>     [cfe-commits] r167788 - [lib/Analysis] Fix bad CFG construction bug
when handling C++ 'try' statements.
>
> Even if it required some heuristics and horrible hackery in the commit
script, it would be worth it.
Yeah, crossed my mind too. First problem being the matching changes in
test/include/lib - but, as you say, some heuristics might catch the
common cases.

2012/11/16 Chris Lattner <clattner at apple.com>:
>
> On Nov 15, 2012, at 2:28 PM, Chandler Carruth <chandlerc at
google.com> wrote:
>
>>> [cfe-commits] r167788 - in /cfe/trunk: lib/Analysis/CFG.cpp
test/Analysis/dead-stores.cpp
>>>
>>> with
>>>
>>> [cfe-commits] r167788 - Fix bad CFG construction bug when handling
C++ 'try' statements.
>>>
>>> The first gives me just enough information (in my abbreviated
mesage list window) to see that there was a change to Clang's Analysis
library; if I look at the whole subject, I can see what files were changed (or
the first few, at least). But the second actually tells me what changed, and
whether or not it affects me.
>>>
>>> Having this form will also encourage people to put a useful summary
in the first line of their commit, which is useful for anyone viewing history
(especially git users, where the one-line summary is very common).
>>>
>>> Is there a reason we currently prefer files to log messages?
>>
>> I suspect not. I would much prefer your suggestion. If you can
>> implement the fix to our scripts (and get Tanya or Chris or Anton to
>> give you access), I would be thrilled.
>
> I'm in favor of it.  Of course, the truly awesomest thing would be
something like:
>
>     [cfe-commits] r167788 - [lib/Analysis] Fix bad CFG construction bug
when handling C++ 'try' statements.
>
> Even if it required some heuristics and horrible hackery in the commit
script, it would be worth it.
+1, awesome. Regardless of putting [path/to/dir], I prefer a summary line.

I know a few person tends to put blank line in 1st line, to see blank
line with git --oneline, though.
It could be improved if he saw blank line from *-commits. :)

...Takumi

On 11/16/2012 02:47 AM, Chris Lattner wrote:
>
> On Nov 15, 2012, at 2:28 PM, Chandler Carruth <chandlerc at
google.com> wrote:
>
>>> [cfe-commits] r167788 - in /cfe/trunk: lib/Analysis/CFG.cpp
test/Analysis/dead-stores.cpp
>>>
>>> with
>>>
>>> [cfe-commits] r167788 - Fix bad CFG construction bug when handling
C++ 'try' statements.
>>>
>>> The first gives me just enough information (in my abbreviated
mesage list window) to see that there was a change to Clang's Analysis
library; if I look at the whole subject, I can see what files were changed (or
the first few, at least). But the second actually tells me what changed, and
whether or not it affects me.
>>>
>>> Having this form will also encourage people to put a useful summary
in the first line of their commit, which is useful for anyone viewing history
(especially git users, where the one-line summary is very common).
>>>
>>> Is there a reason we currently prefer files to log messages?
>>
>> I suspect not. I would much prefer your suggestion. If you can
>> implement the fix to our scripts (and get Tanya or Chris or Anton to
>> give you access), I would be thrilled.
>
> I'm in favor of it.  Of course, the truly awesomest thing would be
something like:
>
>      [cfe-commits] r167788 - [lib/Analysis] Fix bad CFG construction bug
when handling C++ 'try' statements.
Moving the important information first may also be worth considering. 
Subject lines are often cut off.

I prefer

[lib/Analysis] Fix bad CFG construction bug when handing C++ 'try' ...

rather than

[cfe-commits] r167788 - [lib/Analysis] Fix bad CFG construction ...

"[cfe-commits]" can be removed from the subject as the amount of mails
forces people anyway to filter/tag their emails

We can leave "r167788" in the subject line as people probably search
for
it. However, the commit message seems to be better to actually recognize 
a certain commit. So we do not need the revision at the beginning of the 
line.

Cheers
Tobi

I agree that the list name is redundant and should be dropped, but the revision
number is compact and very useful...

-Chris

On Nov 15, 2012, at 6:45 PM, Tobias Grosser <tobias at grosser.es> wrote:
> On 11/16/2012 02:47 AM, Chris Lattner wrote:
>> 
>> On Nov 15, 2012, at 2:28 PM, Chandler Carruth <chandlerc at
google.com> wrote:
>> 
>>>> [cfe-commits] r167788 - in /cfe/trunk: lib/Analysis/CFG.cpp
test/Analysis/dead-stores.cpp
>>>> 
>>>> with
>>>> 
>>>> [cfe-commits] r167788 - Fix bad CFG construction bug when
handling C++ 'try' statements.
>>>> 
>>>> The first gives me just enough information (in my abbreviated
mesage list window) to see that there was a change to Clang's Analysis
library; if I look at the whole subject, I can see what files were changed (or
the first few, at least). But the second actually tells me what changed, and
whether or not it affects me.
>>>> 
>>>> Having this form will also encourage people to put a useful
summary in the first line of their commit, which is useful for anyone viewing
history (especially git users, where the one-line summary is very common).
>>>> 
>>>> Is there a reason we currently prefer files to log messages?
>>> 
>>> I suspect not. I would much prefer your suggestion. If you can
>>> implement the fix to our scripts (and get Tanya or Chris or Anton
to
>>> give you access), I would be thrilled.
>> 
>> I'm in favor of it.  Of course, the truly awesomest thing would be
something like:
>> 
>>     [cfe-commits] r167788 - [lib/Analysis] Fix bad CFG construction bug
when handling C++ 'try' statements.
> 
> Moving the important information first may also be worth considering.
Subject lines are often cut off.
> 
> I prefer
> 
> [lib/Analysis] Fix bad CFG construction bug when handing C++ 'try'
...
> 
> rather than
> 
> [cfe-commits] r167788 - [lib/Analysis] Fix bad CFG construction ...
> 
> "[cfe-commits]" can be removed from the subject as the amount of
mails forces people anyway to filter/tag their emails
> 
> We can leave "r167788" in the subject line as people probably
search for it. However, the commit message seems to be better to actually
recognize a certain commit. So we do not need the revision at the beginning of
the line.
> 
> Cheers
> Tobi
> 
>

Hi,

Chris Lattner wrote:
> 
> On Nov 15, 2012, at 2:28 PM, Chandler Carruth <chandlerc at
google.com> wrote:
> 
> >> [cfe-commits] r167788 - in /cfe/trunk: lib/Analysis/CFG.cpp
test/Analysis/dead-stores.cpp
> >> 
> >> with
> >> 
> >> [cfe-commits] r167788 - Fix bad CFG construction bug when handling
C++ 'try' statements.
> >> 
> >> The first gives me just enough information (in my abbreviated
mesage list window) to see that there was a change to Clang's Analysis
library; if I look at the whole subject, I can see what files were changed (or
the first few, at least). But the second actually tells me what changed, and
whether or not it affects me.
> >> 
> >> Having this form will also encourage people to put a useful
summary in the first line of their commit, which is useful for anyone viewing
history (especially git users, where the one-line summary is very common).
> >> 
> >> Is there a reason we currently prefer files to log messages?
> > 
> > I suspect not. I would much prefer your suggestion. If you can
> > implement the fix to our scripts (and get Tanya or Chris or Anton to
> > give you access), I would be thrilled.
> 
> I'm in favor of it.  Of course, the truly awesomest thing would be
something like:
> 
>     [cfe-commits] r167788 - [lib/Analysis] Fix bad CFG construction bug
when handling C++ 'try' statements.
> 
> Even if it required some heuristics and horrible hackery in the commit
script, it would be worth it.
I see that currently the commit emails contain this marker in the header:
X-Mailer: svnmailer-1.0.9
So the following fixes assume that we are running svnmailer-1.0.9.

To get the first line of the log message as a subject line, please apply this
one line patch: http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=17;bug=379534
Note that you can directly patch the installed script: on my machine it is
located in /usr/local/lib/python2.7/dist-packages/svnmailer/notifier/_mail.py
and then replace or add the following line to the config file
/etc/svn-mailer.conf

commit_subject_template = %(prefix)s r%(revision)s - %(log)s

Also as David Blaikie has pointed out, we are missing an upper bound on the
subject line length, so you may want to consider adding this:

max_subject_length = 120

While we are at it, let's also fix the missing context, by using a custom
diff
command with the -p flag.  Here is the config line:

diff_command = /usr/bin/diff -up -L %(label_from)s -L %(label_to)s %(from)s
%(to)s

Can somebody do these modifications to llvm.org's /etc/svn-mailer.conf?

Thanks,
Sebastian
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The
Linux Foundation

Hi,

Sebastian Pop wrote:
> Chris Lattner wrote:
> > I'm in favor of it.  Of course, the truly awesomest thing would be
something like:
> > 
> >     [cfe-commits] r167788 - [lib/Analysis] Fix bad CFG construction
bug when handling C++ 'try' statements.
> 
> commit_subject_template = %(prefix)s r%(revision)s - %(log)s
I just realized that this line does not match Chris' subject line as it
doesn't
print the directories touched by the patch. Please use the following instead:

commit_subject_template = %(prefix)s r%(revision)s - [%(dirs)s] %(log)s

Thanks,
Sebastian
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The
Linux Foundation