Removed deprecated VariantContextWriterFactory #756

Merged
merged 1 commit into from Dec 13, 2016

Conversation

Projects
None yet
5 participants
Contributor

magicDGS commented Nov 24, 2016 edited

Description

Removed ancient factory for create VariantContextWriter. Found in #707.

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)
@magicDGS magicDGS removed VariantContextWriterFactory
fbb4439

Coverage Status

Coverage increased (+0.1%) to 70.145% when pulling fbb4439 on magicDGS:dgs_remove_VariantContextWriterFactory into de27f18 on samtools:master.

- return AbstractFeatureReader.hasBlockCompressedExtension(location);
- }
-
- public static VariantContextWriter sortOnTheFly(final VariantContextWriter innerWriter, final int maxCachingStartDistance) {
@ronlevine

ronlevine Nov 29, 2016

Contributor

Does sortOnTheFly need to be added to VariantContextWriterBuilder ?

@magicDGS

magicDGS Nov 29, 2016

Contributor

I can add these two static method, but they are just syntactic sugar for the SortingVariantContextWriter constructors. This only requires two change appearances of the following lines of code:

final VariantContextWriter sortingWriter = VariantContextWriterFactory.sortOnTheFly(writer, 1000);
...
final VariantContextWriter sortingWriter = VariantContextWriterFactory.sortOnTheFly(writer, 1000, true);

for these:

final VariantContextWriter sortingWriter = new SortingVariantContextWriter(writer, 1000);
...
final VariantContextWriter sortingWriter = new SortingVariantContextWriter(writer, 1000, true);

I don't think that it is necessary, because code that is using these methods should be refactored anyway. But of course, I could include it.

@ronlevine

ronlevine Nov 29, 2016

Contributor

@magicDGS I agree with you, no need to add it. The builder pattern should not have any static methods plus it the a VERY thin wrapper around SortingVariantContextWriter.

droazen self-assigned this Dec 13, 2016

Contributor

yfarjoun commented Dec 13, 2016

PR in picard ready for this broadinstitute/picard#710 (tests pass)

Contributor

yfarjoun commented Dec 13, 2016

👍

@droazen droazen merged commit 2386a6e into samtools:master Dec 13, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.1%) to 70.145%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment