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

Added Google error-prone to the build and resolved all warnings #239

Merged
merged 9 commits into from
May 23, 2018

Conversation

@codecov
Copy link

codecov bot commented May 22, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##             master   #239   +/-   ##
=======================================
  Coverage       100%   100%           
  Complexity     1016   1016           
=======================================
  Files            90     90           
  Lines          1381   1381           
  Branches        141    141           
=======================================
  Hits           1381   1381
Impacted Files Coverage Δ Complexity Δ
...src/main/java/io/parsingdata/metal/Trampoline.java 100% <ø> (ø) 4 <0> (ø) ⬇️
...ain/java/io/parsingdata/metal/data/ParseGraph.java 100% <100%> (ø) 37 <6> (ø) ⬇️
...java/io/parsingdata/metal/data/ParseReference.java 100% <100%> (ø) 12 <3> (ø) ⬇️
...ain/java/io/parsingdata/metal/data/ParseValue.java 100% <100%> (ø) 13 <3> (ø) ⬇️
...va/io/parsingdata/metal/expression/value/GUID.java 100% <100%> (ø) 9 <2> (ø) ⬇️
...rc/main/java/io/parsingdata/metal/token/Until.java 100% <100%> (ø) 37 <0> (ø) ⬇️
.../java/io/parsingdata/metal/encoding/ByteOrder.java 100% <100%> (ø) 1 <0> (ø) ⬇️

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 54f1fc4...7880b56. Read the comment docs.

Copy link
Contributor

@mvanaken mvanaken left a comment

Choose a reason for hiding this comment

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

😎

stepSize.compareTo(ZERO) > 0 && currentSize.compareTo(maxSize) > 0 ||
stepSize.compareTo(ZERO) < 0 && currentSize.compareTo(maxSize) < 0) {
(stepSize.compareTo(ZERO) > 0 && currentSize.compareTo(maxSize) > 0) ||
(stepSize.compareTo(ZERO) < 0 && currentSize.compareTo(maxSize) < 0)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We have to say "we told you so", but... 😜 😊 👍

pom.xml Outdated
<forceJavacCompilerUse>true</forceJavacCompilerUse>
<!-- maven-compiler-plugin defaults to targeting Java 5, but our
javac only supports >=6 -->
<source>8</source>
Copy link
Contributor

Choose a reason for hiding this comment

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

This version is linked to the property 'maven.compiler.source', right? Maybe create a new property that is e.g. used by both?

Copy link
Contributor Author

@jvdb jvdb May 23, 2018

Choose a reason for hiding this comment

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

Good point! Those properties actually directly set the values for the plug-in (I think 😅), so I just removed both the source and target property setters there to see if they would use those: still works!

pom.xml Outdated
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.3</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Move version to properties?

pom.xml Outdated
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-compiler-javac-errorprone</artifactId>
<version>2.8.3</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Move version to properties?

pom.xml Outdated
<dependency>
<groupId>com.google.errorprone</groupId>
<artifactId>error_prone_core</artifactId>
<version>2.3.1</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Move version to properties? (Sorry, wilde dit in 1comment doen, maar kan de vorige blijkbaar niet meer editen...)

@jvdb jvdb merged commit f432c33 into master May 23, 2018
@jvdb jvdb deleted the error-prone branch May 23, 2018 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Use Google error-prone in the build and resolve its warnings
2 participants