Details
-
Type:
Improvement
-
Status: Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 2.3.0
-
Component/s: build / infrastructure
-
Labels:None
-
Patch Info:Patch Available
Description
Source files may have inadvertent System.out/err.print statements left in source files on commit. Ideally we don't want any of these changes being checked in and a large majority of the time logging facilities should be utilized. That being said, there are a number of cases where we don't have access to a logger and must use System.out/err.
This JIRA will be used to update our checkstyle to scan for unwanted print statements, and fail the build when/if they are encountered.
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
A few comments on your patch :
- Remove the changes to XMLFormatter.java.
- Fix the the spacing on checkstyle.xml.
- Can you move the suppressions.xml so that it is under the openjpa-project module?
- Remove IdentifierUtilImpl from the supressions.xml file, and remove the System.out call from IdentifierUtilImpl:368
Otherwise, the patch looks good.
Fixed patch according to the comment.
Attached is a patch for maven 2.2.1 because maven 3.0.5 uses a different checkstyle-plugin version.
The newly attached patch has full path to the file in order to distinguish different files with the same file names.
Some files have regular expressions instead of the full path because the regex is good enough in telling the difference between the files which have the same name.
Commit 1504673 from Rick Curtis in branch 'openjpa/trunk'
[ https://svn.apache.org/r1504673 ]
OPENJPA-2410 : Detect SystemOut/Err in code at build time. Patch submitted by Di Wu Lau.
Committed revision 1504673 to trunk. Thanks for the patch Di!
I'll note that the code that I committed was somewhat different than your last patch. I decided to go back to allowing a comment to dis/enable checking to allow for exclusions.
Going forward, if you want to have code that has a System.out/err in it you'll need to use the comment '// START - ALLOW PRINT STATEMENTS' to tell the checkstyle plugin to stop checking... and then us the comment '// STOP - ALLOW PRINT STATEMENTS' to re-enable checking. I'll cross post this to the users / dev list and try to get some info on the wiki also.
ie:
// START - ALLOW PRINT STATEMENTS
System.err.println("Not handled " + fmd.getName() + " of type " + fmd.getDeclaredType());
// STOP - ALLOW PRINT STATEMENTS
Thank you Rick for going through all the print statements!
Hi Rick, I think you meant " // STOP - ALLOW PRINT STATEMENTS " instead of " // END - ALLOW PRINT STATEMENTS " in the checkstyle.xml? It is still "END" for right now. In the java files, it is "STOP". This could cause build failure as we set severity=error for the print statements.
Commit 1504861 from Rick Curtis in branch 'openjpa/trunk'
[ https://svn.apache.org/r1504861 ]
OPENJPA-2410 : Minor change to checkstyle suppressions.xml
Commit 1504883 from Rick Curtis in branch 'openjpa/trunk'
[ https://svn.apache.org/r1504883 ]
OPENJPA-2410 : Another minor change to checkstyle.xml
a patch for checkstyle.xml. changing "END" to "STOP".
Hi Rick, if you take a look at my suppressions.xml about suppressing the src\test files, it looks like this: <suppress checks="RegexpSinglelineJava" files=".*src
test.*java"/>. It's the backslash instead of forward slash. you also need to skip the backslash. I think this would solve the problem.
patch for suppressions.xml only.
Commit 1504984 from Rick Curtis in branch 'openjpa/trunk'
[ https://svn.apache.org/r1504984 ]
OPENJPA-2410 : Yet another checkstyle update.
For some reason the changes I've made so far doesn't run correctly on *nix, and also has problems when running on Maven 3.0+.
Unfortunately my linux machine is having problems right now so I can't get a full build to complete... I'm going to try one more commit this evening to try to get things mostly working and try to find some time later this weekend to get things properly cleaned up.
Sorry for anyone that isn't able to build due to this little fiasco.
Commit 1505051 from Rick Curtis in branch 'openjpa/trunk'
[ https://svn.apache.org/r1505051 ]
OPENJPA-2410 : Yet another checkstyle update. Should fix nix issues.
To run on Maven 3.0.5, maven-checkstyle-plugin needs to be version 2.5. It can be changed in the root pom.xml. I haven't found a more compatible solution for both maven 2.2.1 and maven 3.0.5 yet.
I don't think we want to move to Maven 3.x for the sake of the checkstyle. I think I finally have everything working properly now, but apache svn is having some problems so I can't commit the change.
Commit 1505786 from Rick Curtis in branch 'openjpa/trunk'
[ https://svn.apache.org/r1505786 ]
OPENJPA-2410 : Yet another checkstyle change.
I'll note that when running on maven 3.x or newer you'll get the following failure :
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 4.673s
[INFO] Finished at: Sat Aug 03 12:00:11 CDT 2013
[INFO] Final Memory: 30M/60M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.4:checkstyle (default) on project openjpa-lib: Execution default of goal org.apache.maven.plugins:mav
en-checkstyle-plugin:2.4:checkstyle failed. NullPointerException -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginExecutionException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR] mvn <goals> -rf :openjpa-lib
Attaching a patch for 1.2.x and 2.2.x branches. Heath – Can I have you take a look at the patches and give me a thumbs up if these are okay to commit to the service branches. There are very few .java source file changes, and moreso they should be safe changes to make.
Commit 1510994 from Rick Curtis in branch 'openjpa/branches/2.2.x'
[ https://svn.apache.org/r1510994 ]
OPENJPA-2410: Merge checkstyle changes from trunk to 2.2.x .
Commit 1511008 from Rick Curtis in branch 'openjpa/branches/2.1.x'
[ https://svn.apache.org/r1511008 ]
OPENJPA-2410 : Merge checkstyle changes.
Commit 1511055 from Rick Curtis in branch 'openjpa/branches/2.0.x'
[ https://svn.apache.org/r1511055 ]
OPENJPA-2410: Merge checkstyle changes from trunk.
Commit 1511071 from Rick Curtis in branch 'openjpa/branches/1.2.x'
[ https://svn.apache.org/r1511071 ]
OPENJPA-2410: Merge checkstyle changes from trunk.
pom.xml and suppressions.xml are placed in the top level directory.
checkstyle.xml is placed inside openjpa-project directory.