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

Pull #5033: made XMLLogger writer final #5033

Merged
merged 1 commit into from Aug 31, 2017

Conversation

Projects
None yet
3 participants
@rnveach
Member

rnveach commented Aug 31, 2017

Identified at #5025 (comment) ,

We don't support swapping writers during mid-execution of Checkstyle, so we should remove setOutputStream to prevent people from mistaking this. Identified PR was going to make it public by mistake.

I wanted to make 1 constructor call another but I ran into some issues preventing that.

One constructor is deprecated so I don't think the good constructor should be calling the deprecated one.
There is no conversion from boolean to OutputStreamOptions that I can do in one line as required by a constructor call in a constructor unless I make a new method in OutputStreamOptions which would only be used for this purpose.

Eventually deprecated constructor will be removed, so I think this way is ok. If you wish for me to change, let me know.

@romani

romani approved these changes Aug 31, 2017

if CI pass, ok to merge

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Aug 31, 2017

Codecov Report

Merging #5033 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #5033   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         294     294           
  Lines       15974   15971    -3     
  Branches     3619    3619           
======================================
- Hits        15974   15971    -3
Impacted Files Coverage Δ
...ava/com/puppycrawl/tools/checkstyle/XMLLogger.java 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b4c123...bf2c1d6. Read the comment docs.

codecov-io commented Aug 31, 2017

Codecov Report

Merging #5033 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #5033   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         294     294           
  Lines       15974   15971    -3     
  Branches     3619    3619           
======================================
- Hits        15974   15971    -3
Impacted Files Coverage Δ
...ava/com/puppycrawl/tools/checkstyle/XMLLogger.java 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b4c123...bf2c1d6. Read the comment docs.

@romani romani merged commit 07a730c into checkstyle:master Aug 31, 2017

9 checks passed

IDEA Inspections Pull Request (Checkstyle) TeamCity build finished
Details
Shippable Run 4826 status is SUCCESS.
Details
codacy/pr Good work! A positive pull request.
Details
codecov/patch 100% of diff hit (target 100%)
Details
codecov/project 100% (+0%) compared to 9b4c123
Details
continuous-integration/Distelli
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
wercker/build Wercker pipeline passed
Details

@rnveach rnveach deleted the rnveach:xml_final_writer branch Aug 31, 2017

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