Skip to content

Comments

Broke out the static variables so that they can be used separately.#960

Merged
eitanbanks merged 1 commit intomasterfrom
eb_breakout_extensions
Jul 31, 2017
Merged

Broke out the static variables so that they can be used separately.#960
eitanbanks merged 1 commit intomasterfrom
eb_breakout_extensions

Conversation

@eitanbanks
Copy link

Description

I want to use the individual constants outside of the list, so need them broken out.

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)

Copy link
Member

@lbergelson lbergelson left a comment

Choose a reason for hiding this comment

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

👍

@lbergelson
Copy link
Member

@eitanbanks 👍 to merge when tests pass, but can you update the commit name and comment when you merge so it's clear what constants are being split out.

@eitanbanks eitanbanks force-pushed the eb_breakout_extensions branch from 7fd89b9 to 318a69b Compare July 31, 2017 18:27
@codecov-io
Copy link

codecov-io commented Jul 31, 2017

Codecov Report

Merging #960 into master will increase coverage by 0.003%.
The diff coverage is 100%.

@@              Coverage Diff               @@
##             master      #960       +/-   ##
==============================================
+ Coverage     65.57%   65.573%   +0.003%     
+ Complexity     7376      7375        -1     
==============================================
  Files           528       528               
  Lines         31981     31981               
  Branches       5467      5467               
==============================================
+ Hits          20970     20971        +1     
+ Misses         8868      8866        -2     
- Partials       2143      2144        +1
Impacted Files Coverage Δ Complexity Δ
src/main/java/htsjdk/samtools/util/IOUtil.java 30.945% <100%> (ø) 60 <0> (ø) ⬇️
...a/htsjdk/samtools/cram/encoding/rans/Decoding.java 82.143% <0%> (-3.571%) 11% <0%> (-1%)
...dk/samtools/seekablestream/SeekableHTTPStream.java 54.545% <0%> (-1.515%) 9% <0%> (-1%)
...samtools/util/AsyncBlockCompressedInputStream.java 76% <0%> (+4%) 13% <0%> (+1%) ⬆️

public static final String VCF_EXTENSION = ".vcf";
public static final String BCF_EXTENSION = ".bcf";
public static final String COMPRESSED_VCF_EXTENSION = ".vcf.gz";
public static final String[] VCF_EXTENSIONS = new String[] {VCF_EXTENSION, COMPRESSED_VCF_EXTENSION, BCF_EXTENSION};
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need new String[] when you're using an array literal as an initializer. You can write it like this:

 public static final String[] VCF_EXTENSIONS = {VCF_EXTENSION, COMPRESSED_VCF_EXTENSION, BCF_EXTENSION};

Copy link
Author

Choose a reason for hiding this comment

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

Bah, I was just copying what was already there...
Alright, fine.

@@ -87,7 +87,10 @@ public class IOUtil {
public static final long FIVE_GBS = 5 * ONE_GB;

/** Possible extensions for VCF files and related formats. */
Copy link
Contributor

Choose a reason for hiding this comment

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

This only applies to the field VCF_EXTENSION. Maybe it should be moved down to the line before VCF_EXTENSIONS. And you could add javadocs for the other new public fields.


/** Possible extensions for VCF files and related formats. */
public static final String[] VCF_EXTENSIONS = new String[] {".vcf", ".vcf.gz", ".bcf"};
public static final String VCF_EXTENSION = ".vcf";
Copy link
Contributor

Choose a reason for hiding this comment

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

Below I see fields named XXX_FILE_EXTENSION so the names here aren't consistent with them. I'm not sure how important consistency is in this file.

@eitanbanks eitanbanks force-pushed the eb_breakout_extensions branch from 318a69b to 03e8eaa Compare July 31, 2017 18:43
@eitanbanks eitanbanks merged commit 9c6c364 into master Jul 31, 2017
@tfenne tfenne deleted the eb_breakout_extensions branch November 19, 2017 18:19
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