/htsjdk

Kt varient context change #733

Merged
merged 5 commits into from Nov 21, 2016

Conversation

Reviewers
No reviews
Assignees

@yfarjoun yfarjoun

@lbergelson lbergelson

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@yfarjoun @coveralls @lbergelson @kasont
@yfarjoun
Contributor

yfarjoun commented Oct 27, 2016 • edited by lbergelson

Description

making the changes requested in #657

Fixes for broadinstitute/picard#519

Checklist

  • Code compiles correctly
  • New tests covering changes and new functionality
  • All tests passing
  • Extended the README / documentation, if necessary
  • Is not backward compatible (breaks binary or source compatibility)

kasont added some commits Jul 8, 2016

@kasont @yfarjoun kasont RE:As per request broadinstitute/picard#519 the output is null. f148342
@kasont @yfarjoun kasont - responding to review comments instead of kasont

All checks have passed

3 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
coverage/coveralls — Coverage decreased (-0.004%) to 69.73%
Details
be1343b
@yfarjoun
Contributor

yfarjoun commented Oct 27, 2016

@lbergelson this was reviewed by you in a different incarnation...can you check that it does what you want now?

yfarjoun referenced this pull request Oct 27, 2016

Closed

RE:As per request https://github.com/broadinstitute/picard/issues/519… #657

@coveralls

coveralls commented Oct 27, 2016

Coverage Status

Coverage decreased (-0.004%) to 69.73% when pulling be1343b on kt_varientContext_change into 4031fac on master.
@lbergelson
Contributor

lbergelson commented Oct 27, 2016

@yfarjoun It looks right to me, but could you add a unit test? That function is totally untested.

yfarjoun was assigned by lbergelson Oct 27, 2016

@yfarjoun yfarjoun -Added test

All checks have passed

3 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.01%) to 69.743%
Details
9126b2a
@coveralls

coveralls commented Oct 27, 2016

Coverage Status

Coverage increased (+0.05%) to 69.78% when pulling 9126b2a on kt_varientContext_change into 4031fac on master.
@yfarjoun yfarjoun -Added tests

All checks have failed

2 failing checks
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
continuous-integration/travis-ci/push — The Travis CI build failed
Details
c179f22
@yfarjoun
Contributor

yfarjoun commented Oct 27, 2016

done.
@lbergelson
Contributor

lbergelson commented Oct 28, 2016

@yfarjoun One of the tests is failing.
@yfarjoun
Contributor

yfarjoun commented Oct 28, 2016

hmmm. need to figure out why tests are failing!
@yfarjoun yfarjoun - fixed a test that was failing

All checks have passed

3 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.1%) to 69.864%
Details
3c7209d
@coveralls

coveralls commented Nov 7, 2016

Coverage Status

Coverage increased (+0.1%) to 69.864% when pulling 3c7209d on kt_varientContext_change into 4031fac on master.
@yfarjoun
Contributor

yfarjoun commented Nov 16, 2016

@lbergelson fixed test, OK to merge?

lbergelson was assigned by yfarjoun Nov 19, 2016

@lbergelson
Contributor

lbergelson commented Nov 21, 2016

@yfarjoun 👍

@yfarjoun yfarjoun merged commit 55c0ca8 into master Nov 21, 2016
3 checks passed

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.1%) to 69.864%
Details

yfarjoun deleted the kt_varientContext_change branch Nov 21, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment