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

Replaced Contig with ContigName in AlignmentRecord #72

Closed
wants to merge 1 commit into from

Conversation

jpdna
Copy link
Member

@jpdna jpdna commented Apr 2, 2016

Related to issue #983 in adam repo
Pulls 'Contig' out of 'AlignmentRecord' for efficiency issues, and replace it with 'ContigName' which is used as an index into the 'SequenceDictionary' which contains the Contig metadata details

@AmplabJenkins
Copy link

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

@@ -11,7 +11,7 @@

<groupId>org.bdgenomics.bdg-formats</groupId>
<artifactId>bdg-formats</artifactId>
<version>0.7.1-SNAPSHOT</version>
<version>0.7.2-SNAPSHOT</version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.7.1 isn't released yet, is it? Why this version bump?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I see this was to have [https://github.com/bigdatagenomics/adam/pull/988] depend on a unique version it could use.

might be nice to have a test maven server we could upload builds to using their git SHA as their maven version, as is discussed somewhat here. out of scope here though, so nevermind!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, that's a good suggestion, and probably wouldn't be too hard to implement.

@fnothaft
Copy link
Member

fnothaft commented Apr 3, 2016

LGTM! If no one has an objection, I will merge tomorrow PM. Ping here if you'd like me to hold off on merging to review.

@fnothaft
Copy link
Member

fnothaft commented Apr 5, 2016

Thanks @jpdna! Manually merged as 2229115.

@fnothaft fnothaft closed this Apr 5, 2016
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.

4 participants