[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13209727#comment-13209727 ] Avery Ching commented on GIRAPH-40: --- Can another committer please look at this as per Jakob's request? Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Attachments: GIRAPH-40.2.patch, GIRAPH-40.3.patch, GIRAPH-40.patch, GIRAPH-40.patch Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13209801#comment-13209801 ] Avery Ching commented on GIRAPH-40: --- Thanks so much for the reviews Jakob and Sebastian. It's committed. @Sebastian, 'mvn compile' and 'mvn package' will succeed with violations. Anything using 'verify', i.e. 'mvn verify' or 'mvn install' will hit problems with checkstyle and rat. Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Attachments: GIRAPH-40.2.patch, GIRAPH-40.3.patch, GIRAPH-40.patch, GIRAPH-40.patch Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13209814#comment-13209814 ] Hudson commented on GIRAPH-40: -- Integrated in Giraph-trunk-Commit #83 (See [https://builds.apache.org/job/Giraph-trunk-Commit/83/]) GIRAPH-40: Added checkstyle for enforcement of code conventions. All Giraph source files now pass checkstyle. (aching). (Revision 1245205) Result = SUCCESS aching : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVNview=revrev=1245205 Files : * /incubator/giraph/trunk/CHANGELOG * /incubator/giraph/trunk/checkstyle.xml * /incubator/giraph/trunk/license-header.txt * /incubator/giraph/trunk/pom.xml * /incubator/giraph/trunk/src/main/java/org/apache/giraph/GiraphRunner.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/HashMapVertexPageRankBenchmark.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankComputation.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/package-info.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/ApplicationState.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/BspInputFormat.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/BspInputSplit.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/BspOutputFormat.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/BspRecordReader.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/BspRecordWriter.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/ImmutableOutputCommitter.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/SuperstepState.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/package-info.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/ArrayListWritable.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/CommunicationsInterface.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/MsgList.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/RPCCommunications.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/ServerInterface.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/VertexIdMessages.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/VertexIdMessagesList.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/VertexList.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerCommunications.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/package-info.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/ConnectedComponentsVertex.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/GeneratedVertexInputFormat.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/GeneratedVertexReader.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/IntIntNullIntTextInputFormat.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/LongSumAggregator.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/MaxAggregator.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/MinAggregator.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/MinimumIntCombiner.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleAggregatorWriter.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCombinerVertex.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleFailVertex.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMsgVertex.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleShortestPathsVertex.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSumCombiner.java *
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13208731#comment-13208731 ] Jakob Homan commented on GIRAPH-40: --- ok, looks good to me. +1. However, since no one can do a full review of this patch, I'd like another committer to +1 it as well before committing. This helps us to explain away not actually doing a full review. Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Attachments: GIRAPH-40.2.patch, GIRAPH-40.patch, GIRAPH-40.patch Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13208750#comment-13208750 ] Jakob Homan commented on GIRAPH-40: --- Actually, I have a concern: Compiles will now fail if checkstyle guidelines are not met. I tested this and it's true. This means that if you have an extra space in an if statement, you can't compile, even if you're planning to clean up the code later. This is going to be a huge problem. During development the code has to *always* pass checkstyle, not just when submitting a patch. Is there a way to turn this off for compile and just run checkstyle during a specific run? This would mean that it would be up to the submitted and committer to verify correctness, exactly as is required currently with rat... I have to withdraw my -1. Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Attachments: GIRAPH-40.2.patch, GIRAPH-40.patch, GIRAPH-40.patch Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13208786#comment-13208786 ] Jakob Homan commented on GIRAPH-40: --- Thanks. +1 on latest patch, while still hoping to get another committer to take a look. Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Attachments: GIRAPH-40.2.patch, GIRAPH-40.3.patch, GIRAPH-40.patch, GIRAPH-40.patch Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13207185#comment-13207185 ] Jakob Homan commented on GIRAPH-40: --- bq. The below examples are what Checkstyle wants to have us do. So does that mean code not in that hideous style will be flagged by Checkstyle? I'm confused by the next example you posted, which says Checkstyle won't enforce indenting post line wrap... Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Attachments: GIRAPH-40.patch Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13207188#comment-13207188 ] Avery Ching commented on GIRAPH-40: --- So for the first example, we need to follow that format, or else checkstyle will mark it an error. For the second examples, checkstyle doesn't seem to enforce the line wrap indent. So we need to still keep an eye out for those issues. Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Attachments: GIRAPH-40.patch Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13207195#comment-13207195 ] Jakob Homan commented on GIRAPH-40: --- bq. So for the first example, we need to follow that format, or else checkstyle will mark it an error. Blech. -0.9... That's a big change from what we agreed on earlier. Can that particular check be turned off? Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Attachments: GIRAPH-40.patch Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13207203#comment-13207203 ] Avery Ching commented on GIRAPH-40: --- I'm not a checkstyle expert, but I don't think so. I can play around with trying to fix that. Or we can fix in another issue. I should be done with this patch today. Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Attachments: GIRAPH-40.patch Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13207205#comment-13207205 ] Jakob Homan commented on GIRAPH-40: --- OK. If we can fix it later, it'll be less traumatic than the patch coming today since it'll just apply to method signatures... Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Attachments: GIRAPH-40.patch Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13206291#comment-13206291 ] Avery Ching commented on GIRAPH-40: --- Thank you for the feedback Claudio. I'll continue to transition the other files and submit a final patch unless anyone has any objections. Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Attachments: GIRAPH-40.patch Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[
https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13205976#comment-13205976
]
Avery Ching commented on GIRAPH-40:
---
Here are some examples of one problem: Checkstyle doesn't seem to be able to
handle single indent versus double indent of 2 spaces when appropriate. The
below examples are what Checkstyle wants to have us do.
{noformat}
@Override
public BasicVertexLongWritable, DoubleWritable, DoubleWritable, M
getCurrentVertex() throws IOException, InterruptedException {
@Override
public VertexReaderLongWritable, DoubleWritable, DoubleWritable, M
createVertexReader(InputSplit split, TaskAttemptContext context)
throws IOException {
{noformat}
Also, checkstyle won't enforce indenting after a line wrap. So both of these
examples are passing checkstyle.
{noformat}
aggregateVertices =
configuration.getLong(
PseudoRandomVertexInputFormat.AGGREGATE_VERTICES, 0);
aggregateVertices =
configuration.getLong(
PseudoRandomVertexInputFormat.AGGREGATE_VERTICES, 0);
{noformat}
That being said, I think this is the right thing to do and we can make some
sacrifices to have better, more uniform code. Please let me know your thoughts.
Adding checkstyle enforcement of Giraph code conventions
Key: GIRAPH-40
URL: https://issues.apache.org/jira/browse/GIRAPH-40
Project: Giraph
Issue Type: New Feature
Reporter: Avery Ching
Assignee: Avery Ching
Priority: Minor
Attachments: GIRAPH-40.patch
Now that we have some code conventions (see GIRAPH-21), we should enforce
them with a maven checkstyle plugin.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13199806#comment-13199806 ] Claudio Martella commented on GIRAPH-40: I'm pretty much in favor of this, it's making part of contribution process complicated. I think though, to simplify the process, we might/should provide style configs for Eclipse/Netbeans to be used by contributors, or for an external formatting tool (jindent, jalopy, jakobe etc.). Here's a list of tools: http://mindprod.com/jgloss/beautifier.html Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Priority: Minor Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13199938#comment-13199938 ] Avery Ching commented on GIRAPH-40: --- By the way Claudio, I think several IDEs have support for checkstyle (i.e. Eclipse and Intellij). Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Assignee: Avery Ching Priority: Minor Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-40) Adding checkstyle enforcement of Giraph code conventions
[ https://issues.apache.org/jira/browse/GIRAPH-40?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13199935#comment-13199935 ] Avery Ching commented on GIRAPH-40: --- What I meant and am working on, is failing the build when checkstyle errors occur. For now, I am going through and fixing the checkstyle.xml I have and adjusting code, then will submit a patch with the checkstyle.xml and all the warnings fixed. Then, going forward, we will not have to deal with many formatting issues for patches. Well, that's the goal anyway. =) Adding checkstyle enforcement of Giraph code conventions Key: GIRAPH-40 URL: https://issues.apache.org/jira/browse/GIRAPH-40 Project: Giraph Issue Type: New Feature Reporter: Avery Ching Priority: Minor Now that we have some code conventions (see GIRAPH-21), we should enforce them with a maven checkstyle plugin. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
