Anyone out there interested in helping out with a subversion pre-commit hook to: - remove trailing whitespace, - expand tabs to spaces, - detect 80-col violations, as well as detect other style guideline breakage? I just ran into the trailing whitespace problem: Eclipse and other editors like to trim excess whitespace from source. However, when one commits a patch with trailing whitespace removed, the extraneous diffs make reading the patch more difficult. Reply to me privately if you're interested in helping out. -scooter -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090217/07e158e5/attachment.html>
On Feb 17, 2009, at 2:21 PM, Scott Michel wrote:> - remove trailing whitespace, > - expand tabs to spaces,I'd argue for not changing anything, just fail it.
On Tue, Feb 17, 2009 at 2:35 PM, Mike Stump <mrs at apple.com> wrote:> On Feb 17, 2009, at 2:21 PM, Scott Michel wrote: > > - remove trailing whitespace, > > - expand tabs to spaces, > > I'd argue for not changing anything, just fail it. >Trimming whitespace is innocuous, at best. Expanding tabs to spaces, I might be inclined to agree is a 'fail' since weird formatting can result. 80-col violations are absolutely a 'fail'. -scooter -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090217/1d7dfed3/attachment.html>
Hi Scott,> Anyone out there interested in helping out with a subversion pre-commit hook > to: > > - remove trailing whitespace, > - expand tabs to spaces, > - detect 80-col violations, > > as well as detect other style guideline breakage? > > I just ran into the trailing whitespace problem: Eclipse and other editors > like to trim excess whitespace from source. However, when one commits a > patch with trailing whitespace removed, the extraneous diffs make reading > the patch more difficult.from the subversion manual: "While hook scripts can do almost anything, there is one dimension in which hook script authors should show restraint: do not modify a commit transaction using hook scripts. While it might be tempting to use hook scripts to automatically correct errors, shortcomings, or policy violations present in the files being committed, doing so can cause problems. Subversion keeps client-side caches of certain bits of repository data, and if you change a commit transaction in this way, those caches become indetectably stale. This inconsistency can lead to surprising and unexpected behavior. Instead of modifying the transaction, you should simply validate the transaction in the pre-commit hook and reject the commit if it does not meet the desired requirements. As a bonus, your users will learn the value of careful, compliance-minded work habits." Ciao, Duncan.
On Tue, Feb 17, 2009 at 02:21:23PM -0800, Scott Michel wrote:> Anyone out there interested in helping out with a subversion pre-commit hook > to: > > - remove trailing whitespace, > - expand tabs to spaces, > - detect 80-col violations, > > as well as detect other style guideline breakage? > > I just ran into the trailing whitespace problem: Eclipse and other editors > like to trim excess whitespace from source. However, when one commits a > patch with trailing whitespace removed, the extraneous diffs make reading > the patch more difficult. > > Reply to me privately if you're interested in helping out. > > -scooterYet 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. People who don't care are not bothered too much (their code might be changed by the slave), and fascis^H^H^H^H^H^Hpeople who care can quickly find out where the errors are. By setting the tree stable timer for the slave doing the check lower than the slaves doing the actual builds, no extra build is generated in case the code is modified. Beware, some people might try to ddos your buildbot after seeing all their code rewritten, and some other will simply hate you for all the yelling ;-) Julien, -- Julien Lerouge PGP Key Id: 0xB1964A62 PGP Fingerprint: 392D 4BAD DB8B CE7F 4E5F FA3C 62DB 4AA7 B196 4A62 PGP Public Key from: keyserver.pgp.com
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