Optimize imports

[massie]

We have lots of unused imports. This PR cleans them up.

[fnothaft]

+1; however, a nit: this PR breaks the spacing and ordering guidelines we've been using for our imports. Would it be easy to clean up the sorting/ordering issues?

[massie]

We should have a followup PR which adds something like Scala Refactoring to enforce the import guidelines automatically. I ran Intellij's "Optimize Imports" and then scalariform:format to create this PR. Manually reorganizing the imports would be painful.

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/330/
Test PASSed.

[massie]

This looks ready to ship. I'll open an issue for the Scala Refactoring work.

[massie]

Created issue #445.

This look ready to merge @fnothaft ?

[fnothaft]

@massie can you open an issue and update your commit message?

[massie]

Do you mean I should open an issue called "Optimize Imports" and then prefix the commit message with the issue?

If so, what is the point of that process?

[tdanford]

I think Frank is just asking for an issue along the lines of our writeup in the CONTRIBUTING.md document,

Before contributing code to ADAM, check the Github issue tracker. If there is not an open ticket for
what you would like to work on, please open it. When you submit your changes to ADAM, reference 
the issue from the pull request so that it will be closed when your pull request is merged. Also, please
reference the issue number in your commit.

Do we want to revisit the discussion around this process as a whole? I don't think any of us are trying to add unnecessary protocol or rigamarole, but I think the original justification was that it helps to have one place (the "Issues" page) where we can go and see all past, present, and future changes, with notes explaining why (for our own sake later).

[tdanford]

I think just changing this commit message to reference "[ADAM-445]" as its prefix would be enough to get us moving, right Frank?

[massie]

I agree that if someone submits a PR to fix a specific existing issue that they should link the two. However, it seems unnecessary to require all PRs to have an issue.

[tdanford]

Guys, I'm agnostic on this. I don't think it's that big a deal except that I think we should state a plan in our CONTRIBUTING.md doc and stick with it.

[massie]

Putting the ADAM-445 prefix denotes that we've fixed 445, correct? That's isn't the case here.

I agree we need to update the CONTRIBUTING.md document. Let's keep things as light as we can. If a process doesn't make sense -- let's remove it.

[tdanford]

Can you come into IRC Matt? :-D

[tdanford]

I think I misunderstood the extent of #445's suggestion, please disregard that last comment.

I'm off to meet someone for a coffee for a few minutes, but maybe some of us could have a real-time discussion about this. People seem unnecessarily frustrated, that doesn't need to happen.

[fnothaft]

I'd also like to see this PR integrate #445 as well, if possible, esp since this PR breaks the import formatting guidelines in CONTRIBUTING.md:

Please alphabetize your imports. We follow the following approach: first, alphabetize by package name (e.g., org.apache.spark should be before org.bigdatagenomics.adam). Within a project, "lower level" packages should be sorted ahead (e.g., org.apache.spark.SparkContext should be before org.apache.spark.rdd.RDD). Within a single package, sort alphabetically, but put object implicit imports first (e.g., put org.apache.spark.SparkContext._ before org.apache.spark.Logging, and org.apache.spark.Logging before org.apache.spark.SparkContext).

@tdanford this PR doesn't fix #445, it opens #445.

@massie I think that there should be a 1:1 linking between commits and issues. By being consistent with this, it's easy to track down the lineage of a change:

• Why did we do this change? If it was to fix a bug, who reported it and how did the bug manifest itself?

• It's also useful for tracking whether we've got issues that we keep on fixing. E.g., here, we're fixing a recurring problem with code formatting; IMO, if we have issues logged for this, it's easy to identify whether it is a recurring issue, which would indicate that we should change our workflow (in this case, Use tool like Scala Refactoring to enforce import guidelines #445 will resolve the workflow issue)

  Doesn't ASF require a 1:1 link between PRs and JIRA tickets? I personally don't think it adds more overhead than other parts of our workflow (like ensuring that commits are squashed and rebased), but if other people disagree we should discuss that.

[fnothaft]

I do agree that the issue<->commit link seems a bit heavy for small PRs (e.g., #437), but this PR touches 138 files and would (temporarily?) change the format standards we are using in the repo.

[massie]

When we have dozens of large companies contributing large changes to this code base, we might want to enforce a more dogmatic 1:1 link between PR and issues. Often times, issues are used to organize major software releases, deciding what's in and out, tracking progress to a milestone, etc. We are not there yet.

Anyone with any software experience who looks at this commit will understand immediately what it does. It doesn't matter if it touches every file.

[massie]

I've opened #447 so that we can talk about the contribution guidelines there.

This commit looks ready to ship to me. What say you?

[fnothaft]

I'd prefer to see this PR address #445 before merging.

[massie]

What harm are you trying to avoid by not merging this now? This commit removes all the unused imports littering our codebase now. I will work on sorting the imports to everyone's liking in another PR. Is this really a blocker?

[massie]

I'm going to close this for now and open a new pull request later which automates import cleanup and ordering.