On Feb 18, 2009, at 1:41 AM, Julien Lerouge wrote:> Yet another _fun_ way of doing this is to setup a buildbot slave just > for that. The slave can fix minor stuff like tabs and trailing > whitespaces on its own (checking the changes back in), and yell for > things like 80-col violations and whatnot where the changes would > not be > so trivial.If you're going to change anything then this is the best alternative, otherwise I can live with status quo. Do not reject commit just because of formatting issues. It can have serious -ve impact on productivity. To folks who prefers to reject commits due to formatting errors -- You already rely on a some kind of "tool" to make your day to day life easier. [ Most likely you've your editor automatically replacing tabs into spaces. Your terminal window is only 80 col. wide or your editor is displaying a vertical line to warn you about 80 col. and so on... ]. The build bot suggested by Julien is yet another "tool" that accomplishes the same. One the slave bot can use clang static analyzer ... :) - Devang
For the complete truth in advertising, this was pretty much a trial balloon to gauge reaction. I'm not a big fan of rejecting commits for style violations, but the dev guide has certain guidelines regarding formatting and style. And we're all supposed to be good citizens... My biggest nit, however, was contemplating a commit where 80%+ was trailing whitespace trimming. Yeah, my editor happens to practice good hygiene. I could have been a complete *$$**le and committed a global hygiene patch. I only touched the files that I'll end up committing in another day or two. Since there are style rules, I also decided to see how much reaction there would be if they were enforced at commit time. Evidently, it's as popular as a skunk at a garden party. So, it's not something I'm personally looking to invest much time into. -scooter On Wed, Feb 18, 2009 at 8:53 AM, Devang Patel <dpatel at apple.com> wrote:> > On Feb 18, 2009, at 1:41 AM, Julien Lerouge wrote: > > > Yet another _fun_ way of doing this is to setup a buildbot slave just > > for that. The slave can fix minor stuff like tabs and trailing > > whitespaces on its own (checking the changes back in), and yell for > > things like 80-col violations and whatnot where the changes would > > not be > > so trivial. > > If you're going to change anything then this is the best alternative, > otherwise I can live with status quo. > > Do not reject commit just because of formatting issues. It can have > serious -ve impact on productivity. > > To folks who prefers to reject commits due to formatting errors -- You > already rely on a some kind of "tool" to make your day to day life > easier. [ Most likely you've your editor automatically replacing tabs > into spaces. Your terminal window is only 80 col. wide or your editor > is displaying a vertical line to warn you about 80 col. and so > on... ]. The build bot suggested by Julien is yet another "tool" that > accomplishes the same. One the slave bot can use clang static > analyzer ... :) > > - > Devang > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090219/e2dc0829/attachment.html>
On a related note, I wrote a few scripts to detect and correct some types of such style errors, see llvm/utils/lint/* . I also added a function to llvm/utils/vim/vimrc to delete trailing whitespace and highlight existing trailing whitespace -- if anyone's an Emacs-lisp hacker, please add it to the emacs config file as well. Sure, this doesn't enforce anything, but I'm hoping folks will start to use these tools and will over time clean up the style in the entire code base. 2009/2/20 Scott Michel <scooter.phd at gmail.com>> For the complete truth in advertising, this was pretty much a trial balloon > to gauge reaction. I'm not a big fan of rejecting commits for style > violations, but the dev guide has certain guidelines regarding formatting > and style. And we're all supposed to be good citizens... > > My biggest nit, however, was contemplating a commit where 80%+ was trailing > whitespace trimming. Yeah, my editor happens to practice good hygiene. I > could have been a complete *$$**le and committed a global hygiene patch. I > only touched the files that I'll end up committing in another day or two.I've been fixing things on a directory-by-directory basis as I come across style violations while browsing the code. I'm not in favor of a single global change to fix everything everywhere; I think this can be done gradually over time and the diff will be easier to read if it's smaller, so you can verify that the script (or your editor) did not mangle anything. Misha -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090220/ac1fa480/attachment.html>