On 19/02/14 23:50, Juan Pablo Santos RodrÃguez wrote:
Hi,
there is an Eclipse code style file and a a checkstyle file at
$SVN/jspwiki-war/src/main/config/dev
regarding the format itself, I use [#1] it while coding, as I find it more
readable than
if (cond)
{
whatever
}
else
{
whatever_ever
}
so I'm afraid I'm guilty of breaking the code format across the project,
but the above has too much unused vertical space for me. Haven't performed
a mass-code-format-commit because seems mean to me, so I've preferred to
introduce the format changes incrementally.
On one hand I find that the code format needs some updates (screens
nowadays are much wider than a bunch of years back), as it'll improve code
readability and wouldn't mind code-formatting patches (as long as they're
manageable), but on the other hand I agree that there's a big base of
uniform code format. In any case, I don't care specially; if wanted, I can
update the code formatter file with one closer to [#1]'s style
I am a little surprised there have been so few comments, but I take that
as an encouraging sign.
In principle, I agree with all the points made by Ichiro. However, I
don't feel we should be treating an actively-developing open source
project as a finished work of art - it is more like an engineering
project that is still in-progress. After all, most people prefer to fly
in the latest models of Boeing and Airbus planes. Although my own car is
11 years old, most people prefer the design of a modern Toyota or Ford,
not the models of the same name manufactured 20 years ago.
I think there is a consensus for modest changes to code style, and
thankfully no-one wants to be authoritative or obsessive.
I have very little experience with automated tools for managing code
style. I lived with the netbeans defaults until I found it was letting
me write code that broke the tomcat checkstyle rules, which still seem
to be the most rigid I currently have to work with. Nothing is perfect -
I still put my tomcat code through their checkstyle ant task before
making a patch. I never feel the errors I see are pointless, jarring or
overly pedantic.
My original question was based on little knowledge or experience working
with jspwiki. I sort-of hoped someone would say either:
a) "apache asked us to comply with their checkstyle template and we are
working in that direction slowly", or
b) "we already have our own checkstyle standards, but they are difficult
to enforce while in transition to a full apache project".
I've looked briefly at jspwiki-checkstyle.xml and
jspwiki-eclipse-codestyle.xml. I don't know enough about either to
really understand whether they express the same rules in different ways
(or different rules in different ways!). I don't even know how to ask
maven to apply either of them! Can someone enlighten me?
In principle, the project ought to use one tool and rules file.
Obviously, for it to be helpful it couldn't yet be made a mandatory step
in the build or there would never be any progress with the real purpose
of the project!
I suggest we proceed this way:
1. Cross-compare the two existing rules files, identifying common rules,
conflicting rules, and rules present in only one of the files.
2. Agree on a subset of rules that satisfy the 80:20 principle - achieve
the most with the minimum pain.
3. Decide which tool is most harmonious with the existing project build
mechanism. (Eclipse rules are no help to me as a NetBeans user - or vice
versa for Juan Pablo).
4. Document how to turn the validation tool on for an individual file,
or the entire build. (The tool would have to be turned off, or executed
in warning-mode, until the entire source tree satisfies the initial rule
set).
5. Establish a convention that all commits must be either a) pure logic
changes, or b) pure code style changes. (Hybrid commits would lead to
confusion when trying to follow the history of a source file.)
6. See how it goes, then improve and iterate.
In my experience with tomcat, code style changes happen quite rarely,
and are debated in a mature and intelligent manner by everyone - not
just the committers. The project enforces its code style rules on every
build, but the committers are very polite to anyone submitting a patch
that does not conform. Based on my limited experience working on
jspwiki, that shouldn't be hard to emulate!
WDYT?
Brian
br,
juan pablo
[#1] https://paste.apache.org/moc9
On Thu, Feb 13, 2014 at 9:14 AM, Ichiro Furusato
<ichiro.furus...@gmail.com>wrote:
Hi Brian,
IMO, these kinds of discussions are all about aesthetics and almost always
devolve to arguments
that go nowhere, at least nowhere productive.
I know there have been a number of discussions over the many years of this
project, involving
Janne and others. Whether we love the current formatting or not, I would be
loath to let *anyone*
(including myself) go in and modify the code's formatting according to
their own whims. It's simply
not an appropriate thing for a single member of an open source team to do,
unless that team
member happens to have some demonstrable authority in the project that
might permit them to
dictate the aesthetics of the project's code, and then it'd still be
questionable. I for one don't have
any reason to believe your code would be any "tighter" than anyone else's.
This isn't a matter of
anyone's ability, nor is this because nobody has been willing to step up to
the plate, it's a matter
of respect for the work that has been put into the project. This might have
been a useful subject
when the project was still new, but Janne has long ago defended his choices
and I'm fine with that.
So if I had my way I might format it somehow differently than it is now,
and likely different than
you. Yes, there's more vertical whitespace than is necessary, and I don't
happen to like the extra
linebreaks around the curly braces. But it's not my project. Other projects
I've submitted code
to have different "standards". For myself (and in the Neocortext code base)
I try to comply to
the Sun/Java standard. JSPWiki has its own, and it's relatively consistent
across the project. I
think consistency across the entire codebase is more important than any
individual's desires, so
I'm happy to comply to the standards of the project. I'd even advocate
refactoring classes that
didn't match the current formatting to match the rest of the project rather
than introduce some
questionably "improved" formatting in only a few classes.
One of my coworker's quipped a relevant haiku:
Code is poetry
the compiler doesn't care.
So I can't say that "shaking the tree" on a project that's gone through as
much as this one
(going back over a decade) is (frankly) much appreciated.
Nothing personal. Just not a productive conversation IMO.
Ichiro
On Thu, Feb 13, 2014 at 6:12 AM, Brian Burch <br...@pingtoo.com> wrote:
When I first started working on patches for tomcat several years ago, I
found their code standards quite extreme compared to my personal
preferences. As time has passed, I've come to appreciate their rules and
adopted many (not all) for my own projects.
As I've started working on jspwiki, I must confess to a feeling of shock
when seeing some of the classes for the first time. I'll even confess to
doing a "hatchet job" on 3 or 4 before I could feel comfortable reading
the
code and exploring possible changes. Even debugging under netbeans with a
wide screen and small font can be difficult with so little actual code
visible at once.
I don't want to upset anyone who has worked hard on this project, but
what
would be the reaction to some "clean up only" patches?
I'm thinking only of removing multiple spaces (inherited from tab
replacement, or just to align variables of different lengths), removing
trailing white space, removing unnecessary or confusing empty lines,
getting rid of /n between if and {, putting angle brackets around single
line if statements, that sort of thing.
If you all love it how it is, then I'll learn to do so too. On the other
hand, if tighter code is on your wish list, I'd be pleased to help when
working on a class for some other reason. If you already have a
checkstyle
configuration that you'd like to converge on, I'd like to see it.
Sorry for shaking the tree!
Best intentions from...
Brian
