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

Migrate bdg-formats to new adam-formats module. #1689

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@heuermh
Copy link
Member

heuermh commented Aug 28, 2017

No description provided.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Aug 28, 2017

Coverage Status

Coverage increased (+0.02%) to 83.445% when pulling 91eda67 on heuermh:formats-module into 12c56fd on bigdatagenomics:master.

@AmplabJenkins

This comment has been minimized.

Copy link

AmplabJenkins commented Aug 28, 2017

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

@fnothaft

This comment has been minimized.

Copy link
Member

fnothaft commented Sep 5, 2017

I'm -0.5 to moving this here. We don't rev bdg-formats that often, and we'll want bdg-formats to stay even more stable than ADAM after we go to ADAM 1.0.0. Additionally, moving this in here would lose all of the revision control history that we've got in bdg-formats. What're your motivations for the move, @heuermh ?

@heuermh

This comment has been minimized.

Copy link
Member Author

heuermh commented Sep 5, 2017

If bdg-formats doesn't have a life of its own (afaik the only projects that depend on it without also depending on ADAM are mine) and is typically released in lock-step with ADAM, there is not much reason for it to be in a separate repo.

My latest itch is around documentation; I'd like to host javadocs and mapping docs, and it doesn't feel like those merit standing on their own separate from ADAM docs.

Additionally, moving this in here would lose all of the revision control history that we've got in bdg-formats.

I don't know, there may be some git magick to import the source and save history but yeah, the issues and pull requests would get lost.

@fnothaft

This comment has been minimized.

Copy link
Member

fnothaft commented Sep 6, 2017

(afaik the only projects that depend on it without also depending on ADAM are mine)

@ryan-williams as well. With the work on SNAP and bowtie2 in cannoli, I would still like to attempt the dream eventually of using the schemas from C++ land.

I don't know, there may be some git magick to import the source and save history but yeah, the issues and pull requests would get lost.

It appears that there is a way to import the commits (https://stackoverflow.com/a/8396318), but we'd lose the issues and pull request history, which is displeasing.

@ryan-williams

This comment has been minimized.

Copy link
Member

ryan-williams commented Sep 7, 2017

I don't think I currently depend on bdg-formats anywhere that I don't also depend on adam core, fwiw.

However, it seems like bdg-formats would still be released as its own artifact even if it was put into the ADAM repo, so that wouldn't close the door on anyone seeking to depend on formats and not core, right?

Is formats usually released in lockstep with ADAM, though? That wasn't really my impression.

Anyway there's definitely costs to this move and I don't have much insight into what is improved by it, so my drive-by reaction is not very enthusiastic but I defer to y'all. I don't share the posture that things should be put in the same repo by default.

@fnothaft

This comment has been minimized.

Copy link
Member

fnothaft commented Sep 7, 2017

I don't think I currently depend on bdg-formats anywhere that I don't also depend on adam core, fwiw.

I think your sbt infrastructure may pull it in by default? IIRC, I saw it show up in the transitive dependency tree of genome-loci? That may've been an error on my side.

Anyway there's definitely costs to this move and I don't have much insight into what is improved by it, so my drive-by reaction is not very enthusiastic but I defer to y'all. I don't share the posture that things should be put in the same repo by default.

+1 @ryan-williams

@ryan-williams

This comment has been minimized.

Copy link
Member

ryan-williams commented Sep 8, 2017

I think your sbt infrastructure may pull it in by default? IIRC, I saw it show up in the transitive dependency tree of genome-loci? That may've been an error on my side.

That should be untrue on both counts. sbt 'show fullClasspath' | grep formats in hammerlab/loci should (have always) come up blank, as one way to inspect. Definitely lmk if you see otherwise.

@fnothaft

This comment has been minimized.

Copy link
Member

fnothaft commented Sep 8, 2017

@ryan-williams yeah, you're right. I just reran mvn dependency:tree and didn't see the bdg-formats dep this time around. Not sure why I saw it earlier... must've been bleary eyed late at night...

@heuermh heuermh force-pushed the heuermh:formats-module branch from 91eda67 to aa37459 Jan 9, 2018

@heuermh

This comment has been minimized.

Copy link
Member Author

heuermh commented Jan 9, 2018

Rebased.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 9, 2018

Coverage Status

Coverage decreased (-0.1%) to 82.705% when pulling aa37459 on heuermh:formats-module into 71f9d13 on bigdatagenomics:master.

@AmplabJenkins

This comment has been minimized.

Copy link

AmplabJenkins commented Jan 9, 2018

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

@heuermh heuermh force-pushed the heuermh:formats-module branch from aa37459 to 957d627 Mar 21, 2018

@AmplabJenkins

This comment has been minimized.

Copy link

AmplabJenkins commented Mar 21, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.