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

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

On Fri, Nov 30, 2012 at 9:43 AM, Sebastian Pop <spop at codeaurora.org>
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
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

On Nov 30, 2012, at 9:43 , Sebastian Pop <spop at codeaurora.org> 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

David Blaikie wrote:
> On Fri, Nov 30, 2012 at 9:43 AM, Sebastian Pop <spop at
codeaurora.org> 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