RFC: put commit messages in *-commits subject lines?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
41 messages Options
123
Reply | Threaded
Open this post in threaded view
|

RFC: put commit messages in *-commits subject lines?

Jordan Rose
Hi, everyone. I was comparing our commit messages with another commit list I'm on, and I realized (not for the first time) that our subject lines are less than useful. Why don't we have the first line of our commit messages in the subject lines? Compare:

[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?
Jordan
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Chandler Carruth-2
On Thu, Nov 15, 2012 at 2:20 PM, Jordan Rose <[hidden email]> wrote:

> Hi, everyone. I was comparing our commit messages with another commit list I'm on, and I realized (not for the first time) that our subject lines are less than useful. Why don't we have the first line of our commit messages in the subject lines? Compare:
>
> [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.

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

David Blaikie
In reply to this post by Jordan Rose
On Thu, Nov 15, 2012 at 2:20 PM, Jordan Rose <[hidden email]> wrote:

> Hi, everyone. I was comparing our commit messages with another commit list I'm on, and I realized (not for the first time) that our subject lines are less than useful. Why don't we have the first line of our commit messages in the subject lines? Compare:
>
> [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).

Also, having this format would mean we would be less likely to go over
the subject character limit that causes gmail to truncate subjects &
break threading...

So for both/all those reasons, I'd love this.

> Is there a reason we currently prefer files to log messages?
> Jordan
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Sean Silva
> Also, having this format would mean we would be less likely to go over
> the subject character limit that causes gmail to truncate subjects &
> break threading...

yes.please. This is so annoying. Fixing that alone would be worth it.

-- Sean Silva

On Thu, Nov 15, 2012 at 5:30 PM, David Blaikie <[hidden email]> wrote:

> On Thu, Nov 15, 2012 at 2:20 PM, Jordan Rose <[hidden email]> wrote:
>> Hi, everyone. I was comparing our commit messages with another commit list I'm on, and I realized (not for the first time) that our subject lines are less than useful. Why don't we have the first line of our commit messages in the subject lines? Compare:
>>
>> [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).
>
> Also, having this format would mean we would be less likely to go over
> the subject character limit that causes gmail to truncate subjects &
> break threading...
>
> So for both/all those reasons, I'd love this.
>
>> Is there a reason we currently prefer files to log messages?
>> Jordan
>> _______________________________________________
>> cfe-dev mailing list
>> [hidden email]
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Chris Lattner-2
In reply to this post by Chandler Carruth-2

On Nov 15, 2012, at 2:28 PM, Chandler Carruth <[hidden email]> 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
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

David Blaikie
On Thu, Nov 15, 2012 at 5:47 PM, Chris Lattner <[hidden email]> wrote:

>
> On Nov 15, 2012, at 2:28 PM, Chandler Carruth <[hidden email]> 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.

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

NAKAMURA Takumi
In reply to this post by Chris Lattner-2
2012/11/16 Chris Lattner <[hidden email]>:

>
> On Nov 15, 2012, at 2:28 PM, Chandler Carruth <[hidden email]> 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

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Tobias Grosser-5
In reply to this post by Chris Lattner-2
On 11/16/2012 02:47 AM, Chris Lattner wrote:

>
> On Nov 15, 2012, at 2:28 PM, Chandler Carruth <[hidden email]> 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


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Chris Lattner-2
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 <[hidden email]> wrote:

> On 11/16/2012 02:47 AM, Chris Lattner wrote:
>>
>> On Nov 15, 2012, at 2:28 PM, Chandler Carruth <[hidden email]> 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
>
>
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

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

It's the only part of the message that says "r123456", including the
'r', which in my experience makes it *the* go-to for copypasting.

-- Sean Silva

On Thu, Nov 15, 2012 at 10:40 PM, Chris Lattner <[hidden email]> wrote:

> 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 <[hidden email]> wrote:
>
>> On 11/16/2012 02:47 AM, Chris Lattner wrote:
>>>
>>> On Nov 15, 2012, at 2:28 PM, Chandler Carruth <[hidden email]> 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
>>
>>
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Sebastian Pop-7
In reply to this post by Chris Lattner-2
Hi,

Chris Lattner wrote:

>
> On Nov 15, 2012, at 2:28 PM, Chandler Carruth <[hidden email]> 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
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Sebastian Pop-7
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
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

David Blaikie
On Fri, Nov 30, 2012 at 9:43 AM, Sebastian Pop <[hidden email]> wrote:

> 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

For what it's worth, (I think) Chris' suggestion of including the
directories was about including them "smart"ly by removing conceptual
duplicates (lib/foo + include/foo + test/foo) and generally giving a
brief sense of what a change is touching. If the change you have adds
the full (repo-relative) path all the directories without any smart
deduplication then I suspect it's going to easily push the log
description off most mail readers (especially mobile) & reduce the
value of this change.

I suspect we'll want to just stick with revision + log for now.

What's the prefix? There was some discussion of dropping the
[cfe-commits] & similar prefixes. Can that be done? Do we need to get
more buy-in from mailing list users to make sure this won't break
their mail rules (there are other ways to identify mailing list mail
other than the prefix, but I'm just checking).

Do you have some examples of what the format you're suggesting would
look like for various real-world commits?

>
> Thanks,
> Sebastian
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Jordan Rose
In reply to this post by Sebastian Pop-7

On Nov 30, 2012, at 9:43 , Sebastian Pop <[hidden email]> wrote:

> 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

I would love some magic to filter out test/XXX from the subject line if anything in lib/ has been changed...maybe something like this?

[clang]
for_paths = tools/clang
exclude_paths = tools/clang/test
show_nonmatching_paths = yes

[clang_test]
for_paths = tools/clang/test
ignore_if_other_matches = yes

Jordan
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Sebastian Pop-7
In reply to this post by David Blaikie
David Blaikie wrote:

> On Fri, Nov 30, 2012 at 9:43 AM, Sebastian Pop <[hidden email]> wrote:
> > 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
>
> For what it's worth, (I think) Chris' suggestion of including the
> directories was about including them "smart"ly by removing conceptual
> duplicates (lib/foo + include/foo + test/foo) and generally giving a
> brief sense of what a change is touching. If the change you have adds
> the full (repo-relative) path all the directories without any smart
> deduplication then I suspect it's going to easily push the log
> description off most mail readers (especially mobile) & reduce the
> value of this change.
>
> I suspect we'll want to just stick with revision + log for now.

That's fine.  Alternatively we can provide a list of all the dirs that we want
to see appearing and match them like this:

for_paths = .*(?P<SelectDirs>(lib/Analysis|lib/Transforms/Vectorize|lib/Transforms/Scalar)).*

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

>
> What's the prefix? There was some discussion of dropping the
> [cfe-commits] & similar prefixes. Can that be done? Do we need to get
> more buy-in from mailing list users to make sure this won't break
> their mail rules (there are other ways to identify mailing list mail
> other than the prefix, but I'm just checking).
>
> Do you have some examples of what the format you're suggesting would
> look like for various real-world commits?

I suppose that the current subject format is the default of svn-mailer that is
from the documentation:

  %(prefix)s r%(revision)s %(part)s - %(files/dirs)s

I think that this matches exactly what I can see on the commits mailing lists,
except for the %(part)s part that I haven't seen so far: probably we do not
split large emails.

I did some tests on my local machine, and with the patched version of svnmailer,
it does extract correctly the first line of the log message for all the commits
that I have looked at.

Sebastian
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

David Blaikie
Bump for something that would be good to do.

On Fri, Nov 30, 2012 at 10:49 AM, Sebastian Pop <[hidden email]> wrote:

> David Blaikie wrote:
>> On Fri, Nov 30, 2012 at 9:43 AM, Sebastian Pop <[hidden email]> wrote:
>> > 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

Using r172358 as an example (a moderately large commit chosen
otherwise at random), the current commit message subject is:

[llvm-commits] [llvm] r172358 - in /llvm/trunk:
include/llvm/ADT/StringRef.h
include/llvm/Analysis/DOTGraphTraitsPass.h
include/llvm/Object/RelocVisitor.h include/llvm/Support/YAMLTraits.h
lib/Analysis/ValueTracking.cpp lib/MC/ELFObjectWriter.cpp
lib/MC/WinCOFFObjectWriter.cpp lib/Support/APFloat.cpp
lib/Support/DynamicLibrary.cpp lib/Target/R600/AMDGPUSubtarget.h
lib/Target/R600/AMDILDevice.h
lib/Target/R600/AMDILPeepholeOptimizer.cpp
lib/Transforms/IPO/DeadArgumentElimination.cpp (and on and on)

So I assume the new message would be:

[llvm-commits] [llvm] r172358 - Remove redundant 'llvm::' qualifications

Which seems reasonable. (I think the repo prefix is helpful given that
both the Clang and LLVM lists handle mail for multiple repositories
(llvm + compiler-rt + zorg at least on the llvm-commits list, cfe +
libc++ on the cfe-commits list). Unless we could drop the prefix for
the common repo (llvm and cfe respectively) & add it in only for the
other repos)

I wouldn't mind some way to drop the mailing list prefix too, given
how I organize my email - but I don't suppose there's a nice way to do
that that wouldn't interfere with other people's workflows.

>> >
>> > 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

[llvm-commits] [llvm] r172358 - [include/llvm/ADT
include/llvm/Analysis include/llvm/Object include/llvm/Support
lib/Analysis lib/MC lib/Target lib/Transforms ...] Remove redundant
'llvm::' qualifications

I suppose this isn't the best example - of course it'll be across lots
of directories. But even for more common commits (working in one area
- changing headers, library, and tests all together) would contain a
lot of redundant information in the "dirs" list.

>> For what it's worth, (I think) Chris' suggestion of including the
>> directories was about including them "smart"ly by removing conceptual
>> duplicates (lib/foo + include/foo + test/foo) and generally giving a
>> brief sense of what a change is touching. If the change you have adds
>> the full (repo-relative) path all the directories without any smart
>> deduplication then I suspect it's going to easily push the log
>> description off most mail readers (especially mobile) & reduce the
>> value of this change.
>>
>> I suspect we'll want to just stick with revision + log for now.
>
> That's fine.  Alternatively we can provide a list of all the dirs that we want
> to see appearing and match them like this:
>
> for_paths = .*(?P<SelectDirs>(lib/Analysis|lib/Transforms/Vectorize|lib/Transforms/Scalar)).*
>
> commit_subject_template = %(prefix)s r%(revision)s - [%(SelectDirs)s] %(log)s

Could we go one step further and map these paths to names (eg: map
"lib/Analysis", "include/llvm/Analysis", "test/Analysis" -> "Analysis"
and unique the results)? Also, I'd prefer to have the dirs after the
log message I think, though perhaps if we get them short enough it'd
work.

This solution does seem like it could involve a bit of manual work to
update the list of blessed dirs all the time. If we could express it
generically (lib/*, test/*, include/llvm/*) it might be OK, but as it
stands I'm still not sure it's worth the hassle.

>> What's the prefix? There was some discussion of dropping the
>> [cfe-commits] & similar prefixes. Can that be done? Do we need to get
>> more buy-in from mailing list users to make sure this won't break
>> their mail rules (there are other ways to identify mailing list mail
>> other than the prefix, but I'm just checking).
>>
>> Do you have some examples of what the format you're suggesting would
>> look like for various real-world commits?
>
> I suppose that the current subject format is the default of svn-mailer that is
> from the documentation:
>
>         %(prefix)s r%(revision)s %(part)s - %(files/dirs)s
>
> I think that this matches exactly what I can see on the commits mailing lists,
> except for the %(part)s part that I haven't seen so far: probably we do not
> split large emails.
>
> I did some tests on my local machine, and with the patched version of svnmailer,
> it does extract correctly the first line of the log message for all the commits
> that I have looked at.
>
> Sebastian
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Vane, Edwin
Is anybody working on this? If not, where might one start looking to fix this?

-----Original Message-----
From: [hidden email] [mailto:[hidden email]] On Behalf Of David Blaikie
Sent: Monday, January 14, 2013 1:52 PM
To: Sebastian Pop
Cc: [hidden email] Developers; [hidden email]
Subject: Re: [LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?

Bump for something that would be good to do.

On Fri, Nov 30, 2012 at 10:49 AM, Sebastian Pop <[hidden email]> wrote:

> David Blaikie wrote:
>> On Fri, Nov 30, 2012 at 9:43 AM, Sebastian Pop <[hidden email]> wrote:
>> > 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

Using r172358 as an example (a moderately large commit chosen otherwise at random), the current commit message subject is:

[llvm-commits] [llvm] r172358 - in /llvm/trunk:
include/llvm/ADT/StringRef.h
include/llvm/Analysis/DOTGraphTraitsPass.h
include/llvm/Object/RelocVisitor.h include/llvm/Support/YAMLTraits.h lib/Analysis/ValueTracking.cpp lib/MC/ELFObjectWriter.cpp lib/MC/WinCOFFObjectWriter.cpp lib/Support/APFloat.cpp lib/Support/DynamicLibrary.cpp lib/Target/R600/AMDGPUSubtarget.h lib/Target/R600/AMDILDevice.h lib/Target/R600/AMDILPeepholeOptimizer.cpp
lib/Transforms/IPO/DeadArgumentElimination.cpp (and on and on)

So I assume the new message would be:

[llvm-commits] [llvm] r172358 - Remove redundant 'llvm::' qualifications

Which seems reasonable. (I think the repo prefix is helpful given that both the Clang and LLVM lists handle mail for multiple repositories (llvm + compiler-rt + zorg at least on the llvm-commits list, cfe +
libc++ on the cfe-commits list). Unless we could drop the prefix for
the common repo (llvm and cfe respectively) & add it in only for the other repos)

I wouldn't mind some way to drop the mailing list prefix too, given how I organize my email - but I don't suppose there's a nice way to do that that wouldn't interfere with other people's workflows.

>> >
>> > 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

[llvm-commits] [llvm] r172358 - [include/llvm/ADT include/llvm/Analysis include/llvm/Object include/llvm/Support lib/Analysis lib/MC lib/Target lib/Transforms ...] Remove redundant 'llvm::' qualifications

I suppose this isn't the best example - of course it'll be across lots of directories. But even for more common commits (working in one area
- changing headers, library, and tests all together) would contain a lot of redundant information in the "dirs" list.

>> For what it's worth, (I think) Chris' suggestion of including the
>> directories was about including them "smart"ly by removing conceptual
>> duplicates (lib/foo + include/foo + test/foo) and generally giving a
>> brief sense of what a change is touching. If the change you have adds
>> the full (repo-relative) path all the directories without any smart
>> deduplication then I suspect it's going to easily push the log
>> description off most mail readers (especially mobile) & reduce the
>> value of this change.
>>
>> I suspect we'll want to just stick with revision + log for now.
>
> That's fine.  Alternatively we can provide a list of all the dirs that
> we want to see appearing and match them like this:
>
> for_paths =
> .*(?P<SelectDirs>(lib/Analysis|lib/Transforms/Vectorize|lib/Transforms
> /Scalar)).*
>
> commit_subject_template = %(prefix)s r%(revision)s - [%(SelectDirs)s]
> %(log)s

Could we go one step further and map these paths to names (eg: map "lib/Analysis", "include/llvm/Analysis", "test/Analysis" -> "Analysis"
and unique the results)? Also, I'd prefer to have the dirs after the log message I think, though perhaps if we get them short enough it'd work.

This solution does seem like it could involve a bit of manual work to update the list of blessed dirs all the time. If we could express it generically (lib/*, test/*, include/llvm/*) it might be OK, but as it stands I'm still not sure it's worth the hassle.

>> What's the prefix? There was some discussion of dropping the
>> [cfe-commits] & similar prefixes. Can that be done? Do we need to get
>> more buy-in from mailing list users to make sure this won't break
>> their mail rules (there are other ways to identify mailing list mail
>> other than the prefix, but I'm just checking).
>>
>> Do you have some examples of what the format you're suggesting would
>> look like for various real-world commits?
>
> I suppose that the current subject format is the default of svn-mailer
> that is from the documentation:
>
>         %(prefix)s r%(revision)s %(part)s - %(files/dirs)s
>
> I think that this matches exactly what I can see on the commits
> mailing lists, except for the %(part)s part that I haven't seen so
> far: probably we do not split large emails.
>
> I did some tests on my local machine, and with the patched version of
> svnmailer, it does extract correctly the first line of the log message
> for all the commits that I have looked at.
>
> Sebastian
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Jordan Rose
+John C and Daniel, who have access to the servers.

Way back when this thread was first active I asked around off-list where this needs to happen, and Daniel and Tanya said this (respectively):

>> I'd prefer to stick with using either svn-mailer or svnnotify, as those are the ones we use in other places. svnnotify in particular is very full featured, although we would need to install it on the server but that is straightforward.
>>
> Maybe. The server is very out of date, and there are no packages anymore for it.. so if its just one file then that might work ;)
>
> I need to get John C. to update the OS again. He didn't do it and then Zac came early.. so it never got done. But its time and needs to be done. Once thats done, it will allow us to upgrade llvm.org much easier.

Sebastian, maybe you can psas your full config file on to John and Daniel?

Jordan
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

John Criswell-4
On 1/24/13 11:20 AM, Jordan Rose wrote:
> +John C and Daniel, who have access to the servers.
>
> Way back when this thread was first active I asked around off-list where this needs to happen, and Daniel and Tanya said this (respectively):
>
>>> I'd prefer to stick with using either svn-mailer or svnnotify, as those are the ones we use in other places. svnnotify in particular is very full featured, although we would need to install it on the server but that is straightforward.
>>>
>> Maybe. The server is very out of date, and there are no packages anymore for it.. so if its just one file then that might work ;)
>>
>> I need to get John C. to update the OS again. He didn't do it and then Zac came early.. so it never got done. But its time and needs to be done. Once thats done, it will allow us to upgrade llvm.org much easier.

Hrm.  It sounds like we had a "You're waiting for me; I'm waiting for
you" thing happening back in 2012.  Tanya disappeared mid-discussion in
early 2012 (I assume because Zac arrived), and since llvm.org was
running smoothly, I just decided to wait until Tanya contacted me about
it.  It sounds like she might have been waiting on me to bring the
subject up again.

In any event, for the current upgrade plans, Tanya asked me about dates
for the upgrade, and I responded yesterday with a list of potential
dates.  All LLVM admins should be in the loop as I CC'ed that email to
the llvm-admin list.  Admins, if you haven't received that email, please
let me know.  I am currently waiting for a response on a few questions.

-- John T.


> Sebastian, maybe you can psas your full config file on to John and Daniel?
>
> Jordan

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] RFC: put commit messages in *-commits subject lines?

Sebastian Pop-7
In reply to this post by Jordan Rose
Jordan Rose wrote:

> +John C and Daniel, who have access to the servers.
>
> Way back when this thread was first active I asked around off-list where this needs to happen, and Daniel and Tanya said this (respectively):
>
> >> I'd prefer to stick with using either svn-mailer or svnnotify, as those are the ones we use in other places. svnnotify in particular is very full featured, although we would need to install it on the server but that is straightforward.
> >>
> > Maybe. The server is very out of date, and there are no packages anymore for it.. so if its just one file then that might work ;)
> >
> > I need to get John C. to update the OS again. He didn't do it and then Zac came early.. so it never got done. But its time and needs to be done. Once thats done, it will allow us to upgrade llvm.org much easier.
>
> Sebastian, maybe you can psas your full config file on to John and Daniel?

I started looking at this problem in a blind way:

> 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.

If somebody can send me the current config file from the server, I will modify
that, test locally, and send back a patch.  If you decide to switch to svnnotify,
you wouldn't need my fixes, but I would still be happy to help if needed.

Let's get this bug fixed ;-)

Thanks,
Sebastian
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
123