Minor code cleanup and other miscellanea.

[ianbrandt]

Contributing some minor cleanup done whilst familiarizing myself with the codebase.

To eliminate unused imports I used the Eclipse Organize Imports function in bulk, so some reordering was done as well.

[cheddar]

Generally looks like some good cleanup. Do you really have to close the FileOutputStream() if you close the channel returned from getChannel()?

Also, on the pom changes, I noticed it removed a lot of versions, for anything in a sub-module without a version, is it essentially going to be pulling in the version based on what is defined in the parent pom?

I'm also a little scared that we're going to get into import reordering wars between IntelliJ and Eclipse ;).

[ianbrandt]

Thanks for the merge, and sorry for the belated reply to your comment.

Per http://docs.oracle.com/javase/6/docs/api/java/io/FileOutputStream.html#close(), closing a FileOutputStream should close any Channels from its getChannel(). I didn't see any documentation about the other way round though, as it was in the code. It looks like the OpenJDK7 FileChannelImpl will call close() on the parent stream, but I figured the safest thing was to just call it on both.

Exactly right on the POM changes, I DRY'd up all the versions declared by more than one module to the <dependencyManagement> section of the parent POM. I also added missing declarations to the child POMs for all direct dependencies of each given module. I'm not sure how much any of these modules will end up as dependencies themselves, but I think it less brittle to declare all direct dependencies rather than counting on transient declarations from upstream.

Regarding the import ordering, I certainly agree with your concern. I didn't see any IDE metadata, so I wasn't sure what the preferred one was. My goal was to simply cut down on the volume of trivial Eclipse warnings so that significant ones wouldn't be obscured. I'll see if Eclipse can't be configured to match IntelliJ's defaults, or just switch to IntelliJ if that's the better option.

[cheddar]

Cool. I took everything but reverted the import stuff in an attempt to mitigate merge conflicts that people might have. After the fact I realized that I did it wrong (should've pulled your merge into a local repo and revert that one commit before pushing to master) and probably actually created more merge conflicts than would've existed had I kept it.

Anyway, for import optimization, I generally think it's great (and I have IntelliJ configured to optimize things on check-in), but if the only change to a file is import changes and the import isn't currently causing a bug I generally prefer to let it lie. As far as Eclipse/IntelliJ stuff is concerned, I don't think fighting over ordering is much of a concern as long as we stick to the rule where we only optimize imports if and when we've made some other change to the actual code of a file. That way, there will be some substance accompanying the import change and we can trust that both IntelliJ and Eclipse know how to include the correct imports no matter how they are ordered.