Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format Java source files in jung-algorithms/* with google-java-format (#86) #87

Closed
wants to merge 1 commit into from

Conversation

jbduncan
Copy link
Contributor

This PR is the first of many towards resolving #86.

A summary of the changes applied can be found in the commit message.

@jrtom If this PR is too large for you to review, please let me know and I'll split it up. :)

Changes applied:
1. Added braces to a number of if, while, and for statements that were
   missing them, to improve the formatting generated by
   google-java-format.
2. jung-algorithms/src/test/java/edu/uci/ics/jung/algorithms/shortestpath/TestShortestPath.java:
   Adjusted comments for clarity with new formatting.
3. Ran command to format all Java source files:

   `git ls-files | grep -E '\.java$' | xargs -r java -jar \
    google-java-format-1.3-all-deps.jar --replace`

   where google-java-format-1.3-all-deps.jar was obtained from
   https://github.com/google/google-java-format/releases
@jbduncan jbduncan force-pushed the google-java-format branch from 967a9d5 to d4d1663 Compare June 30, 2017 00:05
@jrtom
Copy link
Owner

jrtom commented Jun 30, 2017

There are a couple of things about this PR as currently conceived.

First, active development is taking place on the common.graph branch. I don't plan to release further versions of JUNG based on what's currently in master unless we have to; I'd prefer to focus on getting 3.0 out ASAP.

Second, the common.graph branch is currently undergoing widely-dispersed changes to finish using the Guava common.graph types everywhere, and I'd prefer to avoid other large-scale changes until that's done, just to make the merges easier.

@jbduncan
Copy link
Contributor Author

Okay, that makes a degree of sense to me. Does that mean you'd rather I wait off on applying large-scale changes like this until 3.0 is out of the door?

@jrtom
Copy link
Owner

jrtom commented Jun 30, 2017

I don't intend to wait until 3.0 is out the door for this, just until the JUNG graph classes have been replaced with the common.graph ones. That's probably 90% done at this point.

And I do want to focus on the common.graph branch.

@jbduncan
Copy link
Contributor Author

Okay. If I've understood you correctly, you want me to wait for the common.graph branch (which as you've explained is probably 90% done now) to get merged into master before going through this again, because of difficulties with merging highly differing branches, is that correct?

@jrtom
Copy link
Owner

jrtom commented Jun 30, 2017

Not quite. I'm doing all the development for 3.0 on the common.graph branch (that is, I'm essentially treating it as the master for 3.0 development, and creating branches off that), and the current plan (although I could be persuaded otherwise) is that merging into the master branch will be basically the last thing that happens before releasing 3.0.

(The reason for this is basically in case I do need to release another 2.x version for some reason before 3.0 can come out.)

What I am proposing is that we wait for the common.graph branch to no longer have any remaining references to the original JUNG graph types, and then do the formatting cleanup on the common.graph branch.

@jbduncan
Copy link
Contributor Author

jbduncan commented Jul 1, 2017

Alright, that makes sense to me!

I'm happy to wait until all the references to the original JUNG graph types are gone and Guava's common.graph types are fully adopted before submitting a new PR based off the common.graph branch. I'll close this PR in the meantime.

@jbduncan jbduncan closed this Jul 1, 2017
jbduncan added a commit to jbduncan/jung that referenced this pull request Aug 4, 2017
The Maven plugin https://github.com/coveo/fmt-maven-plugin was used
to do this job. Those files which are still in the process of being
removed or otherwise dealt with for JUNG 3.0 (as described in
jrtom#87) have been left alone to
prevent unnecessary work and merging problems.

The formatting has not yet been tweaked in places where
google-java-format itself cannot automatically do so but which would
be beneficial for readability, like:
1. Removing superfluous empty lines at the start and end of method
   bodies.
2. Adding braces to if, for and while statements.

The formatting has also not been checked to see if javadocs have been
adversely affected.
@jbduncan jbduncan mentioned this pull request Aug 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants