You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by Tom Browder <to...@gmail.com> on 2008/08/22 10:02:49 UTC
Enforcing Code Style
I think this has been discussed before, but I would appreciate any
ideas on how to minimize formatting changes from multiple developers
using different operating systems and different editors (and
settings).
We have inherited old code with GROSS formatting and are cleaning up
the worst as we go. But for the future, when we add more developers,
we would like to preserve what we have improved.
I am thinking of periodically running a script that filters all source
and header files (C++) through a beautifier like bcpp.
Thanks.
-Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by Kevin Grover <ke...@kevingrover.net>.
On Fri, Aug 22, 2008 at 3:05 PM, Tom Browder <to...@gmail.com> wrote:
> On 8/22/08, Kevin Grover <ke...@kevingrover.net> wrote:
> >
> > On Fri, Aug 22, 2008 at 3:02 AM, Tom Browder <to...@gmail.com>
> wrote:
> >
> > >
> > > I think this has been discussed before, but I would appreciate any
> > > ideas on how to minimize formatting changes from multiple developers
> > > using different operating systems and different editors (and
> > > settings).
> ...
> > I would reformat all code upfront, in one step, and commit it with a
> comment
> > along the lines of "Reformated with 'codeformat ARGS and OPTIONS'". If
> > people reformat as they change things, it becomes impossible to tell what
> > really changed. E.g. the actual code changed get eclipsed by the
> > reformatting changes.
>
> Good idea
> .
> > For day-to-day development, you can either _encourage_ developers to use
> the
> > correct style in the future: this will be mostly automatic if your tools
> > (editor) format the way you want.
>
> That's been the problem--no control over editor settings, and folks
> who have auto wrapping line editors that don't care about line
> lengths. Yuch!
>
> > And/or, you can add a pre-commit hook script that looks at each file in a
> > commit: for each type to which your coding standards apply (i.e. source
> ...
> > the same, the script dies with an error string "Source file XXXX is not
> in
> > the correct coding style:" (you could also append the diff text from the
> > compare). If you have a tool that just checks for coding style, you can
> > also use that and die if it complains.
> ...
>
> > The second approach can be a little draconian.
>
> Yeah, but I bet it gets the attention of those with rogue editors.
>
>
I forgot to mention what I usually do. I have a check target in my Makefile
(or build.xml, or whatever) that runs unit tests and also checks source
code. Mostly this runs my Python code through pep8.py, but I could also use
it for C and other code to check style. My older C code was maintained in a
more manual process - if I ever update it, I start to add these checks.
Then, you just need to beat it into your developers heads to not check in
code (to the main trunk at least) that does not cleanly pass the tests and
format checks.
Socially, for people that repeatedly err, put a Dunce Cap over their door or
on their desk (or some other visible marker to point out who is screwing
up). I've heard of groups doing this for changes that break a build - the
CI (Continuous Integration) environment sends out an email so everyone
immediately knows who screwed up, then they are keepers of the dunce cap
until the next person to err.
I'm sort of surprised that there is not some universal code checking tool
out there. It would be a neat idea to have a generic tool that took BNF
grammers, generic and specific code rules, and checked the code. This type
of tool could easily be extended to reformat code and to markup and output
code for inclusing in documentation. It would also be nice to have one,
consistent, tool that worked with everyhing: Ada, C, C++, C#, Java, Perl,
Python, ....
- Kevin
Re: Enforcing Code Style
Posted by Tom Browder <to...@gmail.com>.
On Sat, Aug 23, 2008 at 2:14 PM, Benjamin Smith-Mannschott
<bs...@gmail.com> wrote:
> Tom, it sounds like you're looking for a technical solution to a social
> problem. That may not be the right way to approach the problem.
You're probably right, Ben. Thanks for chiming in.
My situaion is not all that bad. We've had all the developers for a
software package leave, and I and another guy have had to pick up the
ball. We both have been on the sidelines of the project since its
beginning, so we're not novices, and we both have had complaints about
its direction. So now we get to put up or shut up.
We pretty much agree on the coding format we like, so a one-pass
cleanup is acceptable. Then we can both live with a suitable hook to
enforce the standard. We will then expect our new hires to conform,
so the format hook will not be a surprise if it fires.
At least that's what I'm thinking today.
-Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by Brad Stiles <br...@gmail.com>.
Craig McQueen <mc...@edsrd1.yzk.co.jp> wrote:
> Pat Farrell wrote:
>> Benjamin Smith-Mannschott wrote:
>>
>>> Tom, it sounds like you're looking for a technical solution to a
>>> social problem. That may not be the right way to approach the
>>> problem.
>>>
>>
>> I'll be stronger, its the wrong way to approach the social problem.
Maybe it's not a problem? Maybe different code styles when working is
OK, but in the repository it isn't. I know a guy who works like this;
they just have a tool that they are supposed to run before checking it
in. It lets the individual programmer use his or her own prefered
style, but puts it into the repository in the "standard" way.
It's going a little overboard for my taste, but it works for them.
Brad
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by Pat Farrell <pf...@pfarrell.com>.
Craig McQueen wrote:
> Pat Farrell wrote:
>> I'll be stronger, its the wrong way to approach the social problem.
>>
> Distinguish between a rebel-attitude, versus an innocent oversight. The
> latter can be well served with a technical solution.
So the innocent oversight should have a gentle reminder applied.
The rebel attitude requires management to do an attitude adjustment. If
the adjustment is needed repeatedly, call in the death star to solve the
problem permanently.
--
Pat Farrell
http://www.pfarrell.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by Craig McQueen <mc...@edsrd1.yzk.co.jp>.
Pat Farrell wrote:
> Benjamin Smith-Mannschott wrote:
>
>> Tom, it sounds like you're looking for a technical solution to a social
>> problem. That may not be the right way to approach the problem.
>>
>
> I'll be stronger, its the wrong way to approach the social problem.
>
Distinguish between a rebel-attitude, versus an innocent oversight. The
latter can be well served with a technical solution.
Regards,
Craig McQueen
Re: Enforcing Code Style
Posted by Pat Farrell <pf...@pfarrell.com>.
Benjamin Smith-Mannschott wrote:
> Tom, it sounds like you're looking for a technical solution to a social
> problem. That may not be the right way to approach the problem.
I'll be stronger, its the wrong way to approach the social problem.
--
Pat Farrell
http://www.pfarrell.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by Benjamin Smith-Mannschott <bs...@gmail.com>.
On Aug 23, 2008, at 00:05, Tom Browder wrote:
>> For day-to-day development, you can either _encourage_ developers
>> to use the
>> correct style in the future: this will be mostly automatic if your
>> tools
>> (editor) format the way you want.
>
> That's been the problem--no control over editor settings, and folks
> who have auto wrapping line editors that don't care about line
> lengths. Yuch!
>
>> And/or, you can add a pre-commit hook script that looks at each
>> file in a
>> commit: for each type to which your coding standards apply (i.e.
>> source
> ...
>> the same, the script dies with an error string "Source file XXXX is
>> not in
>> the correct coding style:" (you could also append the diff text
>> from the
>> compare). If you have a tool that just checks for coding style,
>> you can
>> also use that and die if it complains.
> ...
>
>> The second approach can be a little draconian.
>
> Yeah, but I bet it gets the attention of those with rogue editors.
Tom, it sounds like you're looking for a technical solution to a
social problem. That may not be the right way to approach the problem.
OTOH I've been burned by Eclipse's defaulting new workspaces to
*mixed* tabs-and-spaces for indentation more often than I care to
admit. *yuck*. A slap on the knuckles with the first commit would
have done me a world of good then.
// ben
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by Tom Browder <to...@gmail.com>.
On 8/22/08, Kevin Grover <ke...@kevingrover.net> wrote:
>
> On Fri, Aug 22, 2008 at 3:02 AM, Tom Browder <to...@gmail.com> wrote:
>
> >
> > I think this has been discussed before, but I would appreciate any
> > ideas on how to minimize formatting changes from multiple developers
> > using different operating systems and different editors (and
> > settings).
...
> I would reformat all code upfront, in one step, and commit it with a comment
> along the lines of "Reformated with 'codeformat ARGS and OPTIONS'". If
> people reformat as they change things, it becomes impossible to tell what
> really changed. E.g. the actual code changed get eclipsed by the
> reformatting changes.
Good idea
.
> For day-to-day development, you can either _encourage_ developers to use the
> correct style in the future: this will be mostly automatic if your tools
> (editor) format the way you want.
That's been the problem--no control over editor settings, and folks
who have auto wrapping line editors that don't care about line
lengths. Yuch!
> And/or, you can add a pre-commit hook script that looks at each file in a
> commit: for each type to which your coding standards apply (i.e. source
...
> the same, the script dies with an error string "Source file XXXX is not in
> the correct coding style:" (you could also append the diff text from the
> compare). If you have a tool that just checks for coding style, you can
> also use that and die if it complains.
...
> The second approach can be a little draconian.
Yeah, but I bet it gets the attention of those with rogue editors.
> As an FYI, another nice reformatter is astyle (Artistic Style). I like it
> because it's standalone, configurable, and runs everywhere.
That's the one I'm thinking of, too. It sure should help us
cross-platform teams.
Thanks, Kevin.
-Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by Kevin Grover <ke...@kevingrover.net>.
On Fri, Aug 22, 2008 at 3:02 AM, Tom Browder <to...@gmail.com> wrote:
> I think this has been discussed before, but I would appreciate any
> ideas on how to minimize formatting changes from multiple developers
> using different operating systems and different editors (and
> settings).
>
> We have inherited old code with GROSS formatting and are cleaning up
> the worst as we go. But for the future, when we add more developers,
> we would like to preserve what we have improved.
>
> I am thinking of periodically running a script that filters all source
> and header files (C++) through a beautifier like bcpp.
>
> Thanks.
>
> -Tom
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: users-help@subversion.tigris.org
>
>
I would reformat all code upfront, in one step, and commit it with a comment
along the lines of "Reformated with 'codeformat ARGS and OPTIONS'". If
people reformat as they change things, it becomes impossible to tell what
really changed. E.g. the actual code changed get eclipsed by the
reformatting changes.
For day-to-day development, you can either _encourage_ developers to use the
correct style in the future: this will be mostly automatic if your tools
(editor) format the way you want.
And/or, you can add a pre-commit hook script that looks at each file in a
commit: for each type to which your coding standards apply (i.e. source
files, not text or other files), you filter it through the re-formatter (to
a temp copy) and then compare that to the original (diff). If they are not
the same, the script dies with an error string "Source file XXXX is not in
the correct coding style:" (you could also append the diff text from the
compare). If you have a tool that just checks for coding style, you can
also use that and die if it complains.
This forces users to use tools that keep code correctly formatted, or to run
it through the filter before commiting.
The second approach can be a little draconian.
As an FYI, another nice reformatter is astyle (Artistic Style). I like it
because it's standalone, configurable, and runs everywhere.
- Kevin
Re: Enforcing Code Style
Posted by Tom Browder <to...@gmail.com>.
On Thu, Aug 28, 2008 at 6:37 AM, John Aldridge
<jo...@informatix.co.uk> wrote:
...
> I have another use case related to this: HTML documents. The problem is that
> we have various HTML editors in use, all of which have different ideas about
> how to layout HTML. The result is that diffs & blame are pretty much
> useless. A custom diff tool might help a bit, but blame will still be a
...
> Apart from just living with the problem, there seem to be a few options:
...
> c) Implement a pre-commit hook which checks that the data is already in some
> standard format as produced by htmltidy, and rejects the commit if not.
>
> It would be nice, though, to be able to run htmltidy automatically as part
> of the commit process. Whilst I wouldn't want to run a pretty printer over
> (e.g.) C++ source code -- people are entitled to have their carefully
> considered code layout respected, IMO -- I think the issues are different
> with HTML where people generally don't look at the raw text even when
> editing a document.
Agree.
> It would be nicest if this could be done server-side, because otherwise
> there might be problems with people using different repositories with
> different canonicalization rules, and because there are otherwise client
> configuration and deployment issues.
John, great idea, I'm on your side--I vote yes!
-Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by Toby Thain <to...@telegraphics.com.au>.
On 28-Aug-08, at 8:37 AM, John Aldridge wrote:
> Tom Browder wrote:
>> I think this has been discussed before, but I would appreciate any
>> ideas on how to minimize formatting changes from multiple developers
>> using different operating systems and different editors (and
>> settings).
>
> I have another use case related to this: HTML documents. ...
> b) Tell people to edit HTML with a text editor. That will eliminate
> random formatting changes, but not everyone is HTML literate, and
> even those who are would welcome some syntax checking assistance!
IMHO this is by far the sanest approach.
Suggestion: Eclipse has a pretty nice HTML editor as part of its Web
Tools Platform. Plus, Subclipse is a very slick integrated front-end
to Subversion, and Eclipse is thoroughly cross-platform :-)
--Toby
>
> c) ...
>
> --
> Cheers,
> John
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: users-help@subversion.tigris.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by John Aldridge <jo...@informatix.co.uk>.
Tom Browder wrote:
> I think this has been discussed before, but I would appreciate any
> ideas on how to minimize formatting changes from multiple developers
> using different operating systems and different editors (and
> settings).
I have another use case related to this: HTML documents. The problem is
that we have various HTML editors in use, all of which have different
ideas about how to layout HTML. The result is that diffs & blame are
pretty much useless. A custom diff tool might help a bit, but blame will
still be a problem.
Apart from just living with the problem, there seem to be a few options:
a) Tell everyone to use a standard HTML editor. It will be difficult to
reach agreement on one which everyone is happy to use and which doesn't
cost too much, though!
b) Tell people to edit HTML with a text editor. That will eliminate
random formatting changes, but not everyone is HTML literate, and even
those who are would welcome some syntax checking assistance!
c) Implement a pre-commit hook which checks that the data is already in
some standard format as produced by htmltidy, and rejects the commit if not.
It would be nice, though, to be able to run htmltidy automatically as
part of the commit process. Whilst I wouldn't want to run a pretty
printer over (e.g.) C++ source code -- people are entitled to have their
carefully considered code layout respected, IMO -- I think the issues
are different with HTML where people generally don't look at the raw
text even when editing a document.
It would be nicest if this could be done server-side, because otherwise
there might be problems with people using different repositories with
different canonicalization rules, and because there are otherwise client
configuration and deployment issues.
--
Cheers,
John
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by Tom Browder <to...@gmail.com>.
On 8/22/08, John Peacock <jo...@havurah-software.org> wrote:
> Tom Browder wrote:
> > That sounds like a good idea. Am I correct that the script would have
...
> 1) "blame" relies on which revision changed which line for its display
> (which has nothing to do with whether the commit was substantial or just
> whitespace);
...
> > Couldn't the script inject a comment somehow?
...
> Comment what? You obvious would note in the commit for the mass reformat
> that this was done, but you will have "lost" the immediately accessible
> line-oriented history of that file (see below).
...
> > > you won't be able to tell who added a feature. If this is acceptable,
...
> Because the first time someone reformats a file, all of those changed lines
> are now attributed to that user, rather than whoever added the lines in the
> first place. Now, it is possible to use blame with a specific revision
> number to go back before the reformat, but it is not nearly as useful.
Thanks, John. Now I understand the white space and formatting issues
better. I haven't used the blame feature because I've not been in a
heavy multi-developer situation locally before.
I'll do some more investigating and revisit this later.
-Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by David Weintraub <qa...@gmail.com>.
On Fri, Aug 22, 2008 at 9:24 AM, John Peacock
<jo...@havurah-software.org> wrote:
> Tom Browder wrote:
>>
>> That sounds like a good idea. Am I correct that the script would have
>> to be run on the user's host?
>
> No, all hook scripts run on the server; the Subversion project itself has
> shied away from client-side scripting (from a security standpoint). You are
> free to make any sort of wrapper around svn that you like, to run any sort
> of cleanup and validation you want, but that is something you need to manage
> all on your own.
I worked with ClearCase which used Client side hooks. The advantage:
You could make changes in the code, comments, file properties, etc.
before a commit took place.
The disadvantage: You could make changes in the code, comments, file
properties, etc. before a commit took place. That meant what the
developer THINKS he checked in isn't what actually got checked in. If
the developer is using a static working directory, the files on the
developer's machine would be different from what was just checked in.
The other disadvantage is attempting to get the hook working on every
single possible client configuration out there. Wrote a hook in Perl
or Python? That has to be installed on the developer's system. Or
maybe it's an incompatible version. Or, maybe a module you use isn't
installed on the developer's machine.
Is that netshare you reference in your script available? Is the "H"
drive mapped to the correct share? Is the developer on a Windows
machine, or a Linux or Mac?
Imagine working with a couple hundred developers and knowing about 25%
each year will get a new machine. Now, you spend most of your days
configuring new machines in order for your hooks to work.
Yes, developers hate it when a commit fails because they forgot to put
some sort of property on a newly created file (like the svn:eol-style
property on shell scripts), but I'll live with a little developer
grumbling if it means I don't have to worry about whether a
developer's system is correctly configured in order to run the hook.
--
David Weintraub
qazwart@gmail.com
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by John Peacock <jo...@havurah-software.org>.
Tom Browder wrote:
> That sounds like a good idea. Am I correct that the script would have
> to be run on the user's host?
No, all hook scripts run on the server; the Subversion project itself
has shied away from client-side scripting (from a security standpoint).
You are free to make any sort of wrapper around svn that you like, to
run any sort of cleanup and validation you want, but that is something
you need to manage all on your own.
> Couldn't svn diff be defined on the user side to ignore
> white-space-only changes?
You can run an external diff program (like GNU diff) that ignores
whitespace changes but that doesn't help with two things:
1) "blame" relies on which revision changed which line for its display
(which has nothing to do with whether the commit was substantial or just
whitespace);
2) many source-code formatters make non-whitespace changes as well (like
moving braces to the next line).
> Couldn't the script inject a comment somehow?
Comment what? You obvious would note in the commit for the mass
reformat that this was done, but you will have "lost" the immediately
accessible line-oriented history of that file (see below).
>
>> you won't be able to tell who added a feature. If this is acceptable, you
>
> I don't understand why.
Because the first time someone reformats a file, all of those changed
lines are now attributed to that user, rather than whoever added the
lines in the first place. Now, it is possible to use blame with a
specific revision number to go back before the reformat, but it is not
nearly as useful.
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by Tom Browder <to...@gmail.com>.
On Fri, Aug 22, 2008 at 5:12 AM, John Peacock
<jo...@havurah-software.org> wrote:
> Tom Browder wrote:
>>
>> I am thinking of periodically running a script that filters all source
>> and header files (C++) through a beautifier like bcpp.
>
> You could do this manually, but not in a hook (automatic). What you /may/
> want to do is investigate whether your beautifier supports commandline
> operation where it set the exit value to non-zero value (but doesn't
> actually change the file) iff it detected reformatting was required. Then
> you could use this in a pre-commit script to verify that the code meets your
> formatting requirements.
That sounds like a good idea. Am I correct that the script would have
to be run on the user's host?
> IMPORTANT NOTE: however you do this, you will wind up performing at least
> some "whitespace only commits" which will mess up your "blame" history (i.e.
Couldn't svn diff be defined on the user side to ignore
white-space-only changes?
Couldn't the script inject a comment somehow?
> you won't be able to tell who added a feature. If this is acceptable, you
I don't understand why.
> may want to mandate that when a developer opens a file that hasn't already
> been reformatted, they should perform the reformat *and nothing else* and
> commit those changes. If you mix reformatting with other code changes, you
> are going to be horribly confused.
>
> The issue of code formatting is pretty close to a religious debate, so I
> hope you are prepared for the inevitable pushback from the "old timers".
I know what you mean--and I'm an old timer.
Thanks, John, for your thoughts and help. And pardon my ignorance of
subversion scripting.
-Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org
Re: Enforcing Code Style
Posted by John Peacock <jo...@havurah-software.org>.
Tom Browder wrote:
> I am thinking of periodically running a script that filters all source
> and header files (C++) through a beautifier like bcpp.
You could do this manually, but not in a hook (automatic). What you /may/ want
to do is investigate whether your beautifier supports commandline operation
where it set the exit value to non-zero value (but doesn't actually change the
file) iff it detected reformatting was required. Then you could use this in a
pre-commit script to verify that the code meets your formatting requirements.
IMPORTANT NOTE: however you do this, you will wind up performing at least some
"whitespace only commits" which will mess up your "blame" history (i.e. you
won't be able to tell who added a feature. If this is acceptable, you may want
to mandate that when a developer opens a file that hasn't already been
reformatted, they should perform the reformat *and nothing else* and commit
those changes. If you mix reformatting with other code changes, you are going
to be horribly confused.
The issue of code formatting is pretty close to a religious debate, so I hope
you are prepared for the inevitable pushback from the "old timers".
John
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org