Skip to content

Don't ignore case in xml validation#1590

Merged
abbiefarr-zz merged 4 commits intomasterfrom
change-ignore-case
Mar 14, 2017
Merged

Don't ignore case in xml validation#1590
abbiefarr-zz merged 4 commits intomasterfrom
change-ignore-case

Conversation

@abbiefarr-zz
Copy link
Copy Markdown
Contributor

@chanseokoh
Copy link
Copy Markdown
Contributor

chanseokoh commented Mar 14, 2017

What about WebXmlScanner? It might be being taken care of in another PR, but it should be here too in any case.

@abbiefarr-zz
Copy link
Copy Markdown
Contributor Author

It's changed in another PR, but I can change it here too

Copy link
Copy Markdown
Contributor

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

LGTM.

It's changed in another PR, but I can change it here too.

Thanks. This PR should be self-contained, independent of other PRs, because for example, other PRs could be abandoned or rolled back. It's also necessary to maintain correct commit history.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 14, 2017

Codecov Report

Merging #1590 into master will increase coverage by 0.23%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #1590      +/-   ##
============================================
+ Coverage     71.04%   71.27%   +0.23%     
- Complexity     1334     1355      +21     
============================================
  Files           236      236              
  Lines          9068     9170     +102     
  Branches        779      802      +23     
============================================
+ Hits           6442     6536      +94     
- Misses         2300     2305       +5     
- Partials        326      329       +3
Impacted Files Coverage Δ Complexity Δ
...ls/eclipse/appengine/validation/PomXmlScanner.java 100% <100%> (ø) 23 <17> (ø)
...ls/eclipse/appengine/validation/WebXmlScanner.java 90% <100%> (ø) 5 <4> (ø)
.../tools/eclipse/test/util/project/ProjectUtils.java 89.58% <0%> (-0.04%) 37% <0%> (+11%)
...ngine/newproject/flex/AppEngineFlexWizardPage.java 0% <0%> (ø) 0% <0%> (ø)
...wproject/standard/AppEngineStandardWizardPage.java 100% <0%> (ø) 3% <0%> (ø)
...ipse/appengine/newproject/AppEngineWizardPage.java 87.12% <0%> (+1.19%) 19% <0%> (+7%)
...clipse/appengine/facets/NonSystemJobSuspender.java 89.18% <0%> (+5.4%) 10% <0%> (+1%)
...d/tools/eclipse/appengine/libraries/BuildPath.java 84.21% <0%> (+7.84%) 9% <0%> (+2%)

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 f2c7220...6c9a6d1. Read the comment docs.

Copy link
Copy Markdown
Contributor

@elharo elharo left a comment

Choose a reason for hiding this comment

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

For a bug like this, we want a test that fails before the PR and passes after.

@abbiefarr-zz abbiefarr-zz merged commit 2360f53 into master Mar 14, 2017
@abbiefarr-zz abbiefarr-zz deleted the change-ignore-case branch March 14, 2017 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants