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

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

Bump for something that would be good to do.

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

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

-----Original Message-----
From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On
Behalf Of David Blaikie
Sent: Monday, January 14, 2013 1:52 PM
To: Sebastian Pop
Cc: cfe-dev at cs.uiuc.edu Developers; llvmdev at cs.uiuc.edu
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 <spop at codeaurora.org>
wrote:
> 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
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
LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev