Chris Lattner
2012-Nov-16 01:47 UTC
[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
David Blaikie
2012-Nov-16 02:00 UTC
[LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
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.
NAKAMURA Takumi
2012-Nov-16 02:12 UTC
[LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
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
Tobias Grosser
2012-Nov-16 02:45 UTC
[LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
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
Chris Lattner
2012-Nov-16 03:40 UTC
[LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
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 > >
Sebastian Pop
2012-Nov-30 08:05 UTC
[LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
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
Sebastian Pop
2012-Nov-30 17:43 UTC
[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)sI 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
Seemingly Similar Threads
- [LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
- [LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
- [LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
- [LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
- [LLVMdev] Testing canaries