Skip to content

Remove non-ascii chars from javadoc. #5936

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

Merged
merged 1 commit into from
May 15, 2019
Merged

Remove non-ascii chars from javadoc. #5936

merged 1 commit into from
May 15, 2019

Conversation

cmnbroad
Copy link
Collaborator

@cmnbroad cmnbroad commented May 14, 2019

Fix #5934. Remove embedded non-ascii chars from the javadoc, i.e., StrandOddsRatio.java had:

) – ln(
0000000 051 040 342 200 223 040 154 156 050 012

@codecov
Copy link

codecov bot commented May 14, 2019

Codecov Report

Merging #5936 into master will decrease coverage by 7.816%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##              master     #5936       +/-   ##
===============================================
- Coverage     86.822%   79.006%   -7.816%     
+ Complexity     32344     30417     -1927     
===============================================
  Files           1993      1993               
  Lines         149460    149460               
  Branches       16502     16502               
===============================================
- Hits          129764    118082    -11682     
- Misses         13679     25628    +11949     
+ Partials        6017      5750      -267
Impacted Files Coverage Δ Complexity Δ
...ender/tools/walkers/annotator/StrandOddsRatio.java 94.737% <ø> (ø) 8 <0> (ø) ⬇️
...itute/hellbender/tools/walkers/mutect/Mutect2.java 80.952% <ø> (ø) 22 <0> (ø) ⬇️
...s/GermlineContigPloidyModelArgumentCollection.java 0% <0%> (-100%) 0% <0%> (-4%)
...mlineContigPloidyHybridADVIArgumentCollection.java 0% <0%> (-100%) 0% <0%> (-3%)
...ments/GermlineCNVHybridADVIArgumentCollection.java 0% <0%> (-100%) 0% <0%> (-3%)
...r/arguments/GermlineCallingArgumentCollection.java 0% <0%> (-100%) 0% <0%> (-4%)
...dorientation/CollectF1R2CountsIntegrationTest.java 0.714% <0%> (-99.286%) 1% <0%> (-14%)
...kers/filters/VariantFiltrationIntegrationTest.java 0.826% <0%> (-99.174%) 1% <0%> (-25%)
.../walkers/bqsr/BaseRecalibratorIntegrationTest.java 1.031% <0%> (-98.969%) 1% <0%> (-7%)
...s/variantutils/VariantsToTableIntegrationTest.java 1.042% <0%> (-98.958%) 1% <0%> (-21%)
... and 196 more

@lbergelson
Copy link
Member

@cmnbroad This looks good but I was thinking it would be good to try to fix the problem in the build script so it treats things as utf-8 always instead of being platform specific. Are you able to reproduce the error locally?

@cmnbroad
Copy link
Collaborator Author

@lbergelson No, I've never seen this issue locally. Declaring the encoding rather than leaving it to the platform sounds like a good idea, but if we declare it to always be UTF-8, these non-ASCII UTF-8 chars would be allowed in the source and javadoc (although I guess thats happening now anyway). Other than the rare case where we deliberately need non-ASCII chars for test purposes or such (like I recently needed in the htsjdk VCF 4.3 PR), it would be nice if we could keep them out of the source, especially for cases like this where the displayed glyph is hardly distinguishable from the intended one.

@lbergelson
Copy link
Member

There have been a few instances like this though. This one was obviously accidental, but things like the correct spelling of @magicDGS actual name seem like reasonable things to be able to include in the source. Also, testing non-ascii characters seems like something that is going to be increasingly common as we support new versions of the spec so it seems like we should learn to avoid this problem...

@cmnbroad
Copy link
Collaborator Author

@lbergelson Its true, it has happened before. I'll make a separate PR to turn on the UTF-8 encoding and let that run on travis.

@cmnbroad cmnbroad merged commit c773a9c into master May 15, 2019
@cmnbroad cmnbroad deleted the cn_fix_javadoc_chars branch May 15, 2019 20:13
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.1.2.0: build fails: unmappable character for encoding ASCII
2 participants