On 20/02/14 23:46, Glen Mazza wrote:
Brian, if you'd like a larger role on the team, best to start submitting
patches on actual code functionality enhancements -- as you gain more
"cred" on the project, you'll naturally gain more influence on matters
such as code style, especially if you become a committer.
Thanks for your comments, Glen.
It isn't my ambition to become a committer for jspwiki, or any more of
the open source projects that I contribute to...
When a project comes into my sphere of interest for some reason
(personal projects or $work), I need to learn about it quickly.
Sometimes I encounter issues (bugs, features, documentation, examples,
whatever) that hinder my progress. At that point I join the users and
dev mailing lists and ask for help. I like to repay that help by
improving the project in a way that helps me too. Eventually, I drift
away because I feel the "score" is even.
I have recently submitted several JIRAs for jspwiki, and also some bug
fixes and new test cases as patches to the trunk.
It was this work that prompted me to (hopefully diplomatically) try to
discover the current philosophy on code style. Frankly, I was hoping
someone would give me the url to a wiki page explaining everything that
I had stupidly not been able to find myself.
I did not want to give the impression of ingratitude, so tried to be
diplomatic and uncritical. As I explained in an earlier post, I work on
several projects and have minimal difficulty debugging and cutting
patches (simple svn diff) for them.
My initial work with jspwiki hasn't been as smooth and simple as I had
hoped and "code style" felt like the most appropriate title. I've been
around long enough to work with most styles (or lack of them). Debugging
unfamiliar code is always more difficult when code style gets in the
way, but that isn't really my point. When I save a source code change
under my IDE, it might appear to be quite simple. However, when I run
"svn diff" the resultant patch files are shocking to behold. I've spent
hours of error-prone manual editing trying to prune diffs so they
contain ONLY the changes I need, and therefore respect the existing
source-code style. Dozens of tabs have been replaced with white space,
newlines changed, etc, and I have to manually edit them all out of the
patch.
Maybe I could disable style enforcement just for the jspwiki project,
but that isn't productive for me. Diffs for all my other projects need
little or no manual editing before submission to the committers.
It would be easy to respond that if I don't like it I can always go
away... but that means I miss an opportunity to help a project that has
helped me. I really am trying to be helpful here, not tread on the toes
of all the committers.
I felt your
arguments were inadvertently weak because AFAICT you didn't specify
which standard you wished to follow (such as Sun/Java) but just what
*you* happened to think was a good coding style -- that's not a standard
though, as everyone has preferences.
I deliberately didn't want to tell you what standards to use - I was
hoping you would tell me what you all prefer...
FYI, what was perhaps missed from this topic was that we had already
agreed several months back that it's fine to use the Sun/Java standards
that you see on most Apache projects today in our code. (I've moved
maybe a few dozen classes over to the new standard while making other
changes to the code.) I don't recommend changing the code just for the
sake of changing formatting, but while making code functionality
improvements/fixes in particular classes, sure, then it would be fine to
update to the standard Sun/Java coding style as part of a patch.
Good to know. I'll plead incompetence for not finding that discussion.
I did some research and discovered the main project's pom.xml already
defines the maven-eclipse-plugin (surprisingly NOT the
maven-checkstyle-plugin). It refers to BOTH
jspwiki/jspwiki-war/src/main/config/dev/jspwiki-checkstyle.xml. I hadn't
noticed this until now, because my netbeans IDE isn't interested in
eclipse features.
However, I was impressed that "mvn checkstyle:checkstyle" ran OK for me
from a command line:
[INFO]
[INFO]
------------------------------------------------------------------------
[INFO] Building Apache JSPWiki Main War 2.10.1-SNAPSHOT
[INFO]
------------------------------------------------------------------------
[WARNING] The POM for org.eclipse.m2e:lifecycle-mapping:jar:1.0.0 is
missing, no dependency information available
[WARNING] Failed to retrieve plugin descriptor for
org.eclipse.m2e:lifecycle-mapping:1.0.0: Plugin
org.eclipse.m2e:lifecycle-mapping:1.0.0 or one of its dependencies could
not be resolved: Failed to read artifact descriptor for
org.eclipse.m2e:lifecycle-mapping:jar:1.0.0
[INFO]
[INFO] --- maven-checkstyle-plugin:2.11:checkstyle (default-cli) @
jspwiki-war ---
[INFO]
[INFO] There are 52241 checkstyle errors.
[WARNING] Unable to locate Source XRef to link to - DISABLED
[INFO]
------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO]
------------------------------------------------------------------------
I had to run it inside the jspwiki-war sub-project because there are
many lifecycle-mapping complaints within the other sub-projects.
I submitted a change to TestEngine recently, which Harry kindly
committed for me. When I generated the patch I hadn't yet realised that
netbeans would "clean up" a lot of style "errors", and nor had Harry!
Somewhat surprisingly, the latest TestEngine does not produce ANY of
those 52K checkstyle errors.
My latest patch for VersioningFileProvider took me a long time to create
manually. I'm quite relieved to find that checkstyle reports very many
errors against this class!
Note, I also share your heavy dislike for the old-style formatting used
in the majority of the classes still today. That's not the issue here.
I agree.
Are both of the existing checkstyle rule files really necessary? Is it
fair to say they together represent the code style wishes of the committers?
If yes, I would be very happy to submit style-only patches for the
classes I've worked on. Those classes would then disappear from the
reports and take you a small step nearer to making the rules mandatory
in the future.
Thanks for all the hard work!
Brian
Regards,
Glen
On 02/20/2014 02:04 PM, Brian Burch wrote:
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
