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

pom.xml reformatting and minor refactoring #9

Closed
wants to merge 3 commits into from

Conversation

heuermh
Copy link
Member

@heuermh heuermh commented Sep 2, 2014

After this, I still have a few questions:

  • Should jdk.version be 1.7? I don't think this will run on jdk 1.6 due to dependencies that require 1.7.
  • Should the ADAM dependency version be bumped to 0.13.1-SNAPSHOT?
  • Can some of the plugin configuration be moved from module poms to parent? Things like <source>${project.build.sourceDirectory}/../gen-java</source> would lead me to think not.

I'll try irc.

@heuermh
Copy link
Member Author

heuermh commented Sep 2, 2014

I think raw.github.com does a redirect, I just copied the URL after clicking on the Raw button from
https://github.com/bigdatagenomics/bdg-services/blob/master/LICENSE

@fnothaft
Copy link
Member

fnothaft commented Sep 2, 2014

Thanks @heuermh! This looks great! I agree that jdk.version should be 1.7. I would hold off on bumping to ADAM 0.13.1-SNAPSHOT from this PR, as there are/may be more extensive changes—I've opened up #10 for that.

I'm not sure about the plugin configuration bits; cc'ing @carlyeks and @tdanford for that... My inkling is that the plugin configuration probably can't move, but some of this seems to be thrift specific, and is a bit out of the scope of my knowledge.

Also, I'm not sure if you plan to squash these down to one commit before you merge or not (I think 34d3727 should be kept separate from the rest, but am otherwise not picky), but can you prefix the commit(s) with [bdg-services-9]? We're trying to make it easy to track tickets <-> commits.

@heuermh
Copy link
Member Author

heuermh commented Sep 2, 2014

Yep, squashing and adding [bdg-services-9] sounds like a good idea.

/me scrambles to re-read git manual

@heuermh
Copy link
Member Author

heuermh commented Sep 2, 2014

@fnothaft just curious, [bdg-services-9] is kind of a chicken-and-egg thing; I can't add that to my commit messages before I create the pull request with that label.

@fnothaft
Copy link
Member

fnothaft commented Sep 2, 2014

IIRC, the issue tracker should show both open issues and pull requests, so you can just pick the "next" number.

@fnothaft
Copy link
Member

fnothaft commented Sep 2, 2014

Ideally, there's already an issue you're fixing that you can tag. E.g., the fix for #10 will probably show up in PR11, but the commit should reference [bdg-services-10].

@heuermh
Copy link
Member Author

heuermh commented Sep 2, 2014

Yep, create a new issue before every pull request is probably the right thing to do.

@heuermh
Copy link
Member Author

heuermh commented Sep 2, 2014

I'll close this without merge and then create new issues with individual pull requests.

@heuermh heuermh closed this Sep 2, 2014
@heuermh heuermh deleted the pom-refactoring branch November 17, 2014 22:02
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