Vane, Edwin
2013-Jan-24 13:28 UTC
[LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
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)sUsing 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)sCould 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
Jordan Rose
2013-Jan-24 17:20 UTC
[LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
+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
John Criswell
2013-Jan-24 19:41 UTC
[LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
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
Sebastian Pop
2013-Jan-25 20:59 UTC
[LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?
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
Maybe Matching 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] [cfe-dev] RFC: put commit messages in *-commits subject lines?