Andrea Aime wrote:
Gabriel Roldan ha scritto:
Hi guys,
It is a long time since I'm noticing an increasing loose of care on applying the code formatting we're supposed to. Not to mention tabs instead of spaces for indentation, lack of (c) headers, and even small but bothering breaks of the code conventions, like lack of braces in if statements. And also unused imports..
Agreed, that's annoying (quite a bit of the tabs are my fault, when
I switched to Linux I forgot to setup again the GeoTools code convention).
As for lack of braces, can you confirm that's part of the Sun Java
coding convention?
yeah, it is: <http://java.sun.com/docs/codeconv/html/CodeConventions.doc6.html#449>
I really hate to add extra braces if not necessary,
to me:
if(xxx)
// one liner
reads much better than
if(xxx) {
// one liner
}
Our Eclipse formatter does not turn the first form into the second,
so I guess it is actually not part of the convention (either that
or the template is incomplete)
looks like the template is incomplete 
One thing that is annoying me instead is adding spaces in function calls, something like, f( a, b ) instead of f(a,b).
I don't know if the coding convention covers it, for sure the
Ecilpse formatter turns the first form into the latter.
Some of the conventions were against my personal preferences too, but I've choose to stick to them back when the convention was introduced in geotools, as we wanted to have a common ground. Well, sort of, cause I was always as bad as you know at cleaning up javadocs and hence the huge amount of "DOCUMENT ME!" entries still there that Jaloppy loved to add...
Question is, is this deliberate? I bet not and would like to encourage all developers to systematically apply the formatting and follow conventions as described in <http://geoserver.org/display/GEOSDOC/1+Code+Conventions>\.
As we all use eclipse and the formatter is there please just try to get used to press CTRL+ALT+F for formatting and CTRL+ALT+O for imports clean up... please?
Hmmm... I always clean imports, but never user CTRL-ALT-F because it
mixes together real changes with formatting ones.
That's right, hence when editing an existing file that doesn't already have the formatting applied, what I do is to select the new code and press CTRL+ALT+F so it formats only the selection instead of the whole file. That's a lot quicker than trying to stick to the conventions while typing.
If a file is really screwed up, I'd suggest to format it in both
branches (formatting just one is murder, makes porting patches
a nightmare) and make the commit a pure formatting one (no other changes).
I would like that too, wonder if jdeolive's concern about loosing blame ability is stronger than this one though...
Ideally we could mass reformat the code in a moment where there
is just one active branch, but in practice that never happens
(with RC1 we'll be cutting the 2.0.x branch, or at least
we should, and the 1.7.x branch is still active)
Yeah, I'd say we could live with a mid ground solution:
- apply full class formating only to classes not present in 1.7.x
- apply organize imports everywhere, that doesn't hurt blame ability
- commit to apply the formatting to any new code, when patching an existing class, format the patched section.
Cheers,
Gabriel
Cheers
Andrea
