Skip to content
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

JENKINS-52477 - XML Parsers should use report encoding given in XML file #146

Merged
merged 2 commits into from
Apr 9, 2019

Conversation

micansid
Copy link
Contributor

@micansid micansid commented Apr 3, 2019

Detect charset to use from xml header, when not specified in the FileReaderFactory constructor. Added a FileReaderFactory constructor without a charset parameter.

@codecov
Copy link

codecov bot commented Apr 3, 2019

Codecov Report

Merging #146 into master will increase coverage by 0.36%.
The diff coverage is 94.73%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #146      +/-   ##
============================================
+ Coverage     88.11%   88.47%   +0.36%     
- Complexity     1218     1232      +14     
============================================
  Files           158      158              
  Lines          3904     3923      +19     
  Branches        425      427       +2     
============================================
+ Hits           3440     3471      +31     
+ Misses          308      295      -13     
- Partials        156      157       +1
Impacted Files Coverage Δ Complexity Δ
...java/edu/hm/hafner/analysis/FileReaderFactory.java 80% <94.73%> (+80%) 11 <9> (+11) ⬆️
...ain/java/edu/hm/hafner/analysis/ReaderFactory.java 68.29% <0%> (+17.07%) 11% <0%> (+3%) ⬆️

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 6e46226...66684ec. Read the comment docs.

Copy link
Member

@uhafner uhafner left a comment

Choose a reason for hiding this comment

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

Well done!

Can you please add a line to the ChangeLog?

private Charset detectCharset(final InputStream inputStream) throws IOException {
Charset result = null;

try {
Copy link
Member

Choose a reason for hiding this comment

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

Simpler:

        try (Reader reader = new InputStreamReader(inputStream)) {

catch (XMLStreamException e) {
// Charset couldn't be detected
}
inputStream.close();
Copy link
Member

Choose a reason for hiding this comment

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

Can be removed if try-with-resources is used.

@uhafner uhafner merged commit 14aaebe into jenkinsci:master Apr 9, 2019
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.

2 participants