[ http://issues.apache.org/jira/browse/JCR-97?page=comments#action_12457326
]
Jukka Zitting commented on JCR-97:
----------------------------------
The jndi patch seems better, though some of the above concerns still apply.
There are also a few more specific issues:
1) There are cases where you've re-folded Javadocs that don't exceed the 80
character line length. I don't see the value of doing that. Note that every
change has a price in making it harder to track the change history (especially
svn blame is badly affected).
2) Incorrect folding of a comment:
Reference ref = new Reference(BindableRepository.class.getName(),
- BindableRepositoryFactory.class.getName(),
- null); // no classpath defined
+ BindableRepositoryFactory.class.getName(), null); // no
+ //
classpath
+ // defined
In this case the "no classpath defined" comment applies directly to the last
null argument. Thus it is much clearer if the null is on a line of it's own
with the comment trailing.
The lock patch doesn't seem to introduce new kinds of issues, but it also has a
number of problems as outlined above for the other patches.
-1 on the three patches, please revise
> Improve Checkstyle conformance
> ------------------------------
>
> Key: JCR-97
> URL: http://issues.apache.org/jira/browse/JCR-97
> Project: Jackrabbit
> Issue Type: Improvement
> Reporter: Jukka Zitting
> Priority: Minor
> Attachments: jackrabbit.core.jndi.patch, jackrabbit.core.lock.patch,
> jackrabbitAPICheckstylePatch.patch, jackrabbitCoreClusterCleanup.patch,
> jackrabbitCoreConfigCleanup.patch, jackrabbitCoreFsDbCleanup.patch,
> jackrabbitCoreFsDbCleanup.patch, jackrabbitCoreUnnecessaryCodeCleanup.patch
>
>
> This is an ongoing meta-issue for improving the Checkstyle conformance of the
> Jackrabbit codebase.
> Checkstyle (http://checkstyle.sourceforge.net/) is an automated tool for
> checking conformance with coding standard and good coding style. A Checkstyle
> report for Jackrabbit can be generated by running "maven checkstyle".
> Currently the Jackrabbit Checkstyle report contains thousands of trivial
> problems like unused imports and minor formatting issues. While it would be
> possible to just remove those checks from the Jackrabbit Checkstyle
> configuration, it would certainly be better to fix the real issues. After
> fixing the trivial problems, the Checkstyle reports become much more valuable
> tools in locating troublesome code and identifying chances for improvement.
> While this issue remains open, you have an open mandate to improve the
> standards conformance and coding style of the Jackrabbit sources. This
> mandate applies only to changes that fix problems reported by Checkstyle
> while making no changes to the external interface or behaviour of the changed
> code.
> The commit messages of such Checkstyle improvements should be labeled with
> the Jira key of this issue (JCR-97) to mark the changes as style-only. This
> way other committers will have easier time reviewing your changes for bugs or
> other unexpected side-effects.
> If you are not a Jackrabbit committer, but want to help improve the
> Checkstyle conformance, you can make your changes using sources from
> anonymous subversion and send your changes as an attachment to this issue.
> Please see the Javadoc improvement issue JCR-73 for details.
> PS. Blind conformance to style guides is seldom beneficial. Please remember
> that the goal of this issue is to improve Jackrabbit code, not just the
> Checkstyle output!
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
