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

Drop guava runtime dependency #322

Merged
merged 1 commit into from
Jun 13, 2017
Merged

Conversation

simon04
Copy link
Contributor

@simon04 simon04 commented May 23, 2017

Some fuctionality is provided by Java 8, the remaining parts can easily be rewritten.

@majst01
Copy link
Collaborator

majst01 commented May 25, 2017

In general i agree with this, but this PR has to much modifications, changes formatting of lines which are not affected, error messages are different, tests are failing.

If you are trying to improve this i can accept tis.

@simon04 simon04 force-pushed the no-guava branch 2 times, most recently from 7f022a6 to f33d3fb Compare May 25, 2017 07:43
@simon04
Copy link
Contributor Author

simon04 commented May 25, 2017

I just fixed the checkstyle issues (which caused the tests to fail). There shouldn't be any unrelated formatting changes, though.

@codecov-io
Copy link

codecov-io commented May 25, 2017

Codecov Report

Merging #322 into master will increase coverage by 1.05%.
The diff coverage is 87.75%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #322      +/-   ##
============================================
+ Coverage     79.54%   80.59%   +1.05%     
- Complexity      140      147       +7     
============================================
  Files            13       14       +1     
  Lines           743      737       -6     
  Branches         77       67      -10     
============================================
+ Hits            591      594       +3     
+ Misses          109      107       -2     
+ Partials         43       36       -7
Impacted Files Coverage Δ Complexity Δ
src/main/java/org/influxdb/dto/Pong.java 75% <0%> (+25%) 4 <0> (ø) ⬇️
...rc/main/java/org/influxdb/impl/BatchProcessor.java 98.13% <100%> (ø) 17 <0> (ø) ⬇️
src/main/java/org/influxdb/dto/BatchPoints.java 68.83% <100%> (+1.29%) 16 <0> (ø) ⬇️
src/main/java/org/influxdb/dto/Point.java 83.66% <100%> (+0.43%) 26 <3> (+2) ⬆️
src/main/java/org/influxdb/impl/InfluxDBImpl.java 82.12% <100%> (-0.18%) 42 <5> (-1)
src/main/java/org/influxdb/InfluxDBFactory.java 71.42% <62.5%> (+28.57%) 4 <3> (ø) ⬇️
src/main/java/org/influxdb/impl/Preconditions.java 75% <75%> (ø) 6 <6> (?)

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 fb89153...1cefe91. Read the comment docs.

@majst01
Copy link
Collaborator

majst01 commented May 25, 2017

Ok looks better, i wait til 2.6 is published to maven repo and we can then consider merging for the next release.

@simon04 simon04 force-pushed the no-guava branch 3 times, most recently from 867aa22 to bbb28ab Compare May 25, 2017 08:08
@simon04
Copy link
Contributor Author

simon04 commented May 25, 2017

Thanks for reviewing this PR and maintaining this Java project! 👍

@majst01
Copy link
Collaborator

majst01 commented Jun 13, 2017

@simon04 would you please resolve the merge conflict, after that i will merge.

@simon04
Copy link
Contributor Author

simon04 commented Jun 13, 2017

Done.

@majst01 majst01 merged commit 825dad9 into influxdata:master Jun 13, 2017
majst01 added a commit that referenced this pull request Jun 13, 2017
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.

None yet

3 participants