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

Cleaner version of #684 #702

Closed
wants to merge 3 commits into from
Closed

Conversation

BrentonPoke
Copy link

This is to address a request from @fmachado for a new pull request with just the changes involving newline characters in tags. Carriage returns are also handled by the regular expression.

@codecov-commenter
Copy link

codecov-commenter commented Sep 25, 2020

Codecov Report

Merging #702 into master will increase coverage by 1.05%.
The diff coverage is 85.42%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #702      +/-   ##
============================================
+ Coverage     59.09%   60.14%   +1.05%     
- Complexity      413      420       +7     
============================================
  Files            69       70       +1     
  Lines          2540     2602      +62     
  Branches        268      271       +3     
============================================
+ Hits           1501     1565      +64     
+ Misses          972      968       -4     
- Partials         67       69       +2     
Impacted Files Coverage Δ Complexity Δ
src/main/java/org/influxdb/BuilderException.java 100.00% <ø> (ø) 1.00 <0.00> (ø)
src/main/java/org/influxdb/InfluxDB.java 100.00% <ø> (ø) 0.00 <0.00> (ø)
src/main/java/org/influxdb/InfluxDBFactory.java 100.00% <ø> (ø) 6.00 <0.00> (ø)
...ain/java/org/influxdb/InfluxDBMapperException.java 33.33% <ø> (ø) 1.00 <0.00> (ø)
src/main/java/org/influxdb/dto/Pong.java 66.66% <ø> (ø) 5.00 <0.00> (ø)
src/main/java/org/influxdb/dto/QueryResult.java 96.29% <ø> (ø) 6.00 <0.00> (ø)
...rc/main/java/org/influxdb/impl/InfluxDBMapper.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
src/main/java/org/influxdb/impl/Preconditions.java 75.00% <0.00%> (ø) 9.00 <0.00> (ø)
.../influxdb/msgpack/MessagePackConverterFactory.java 100.00% <ø> (ø) 3.00 <0.00> (ø)
...ava/org/influxdb/msgpack/QueryResultModelPath.java 90.00% <ø> (ø) 7.00 <0.00> (ø)
... and 27 more

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 6107962...e1af327. Read the comment docs.

@fmachado
Copy link
Contributor

@BrentonPoke thanks again for your contribution!

When you create a PR, it's a good practice to limit your changes only to the issue you are fixing/improving.

Quoting a Gist that I found https://gist.github.com/mikepea/863f63d6e37281e329f8

Try to only fix one issue or add one feature within the pull request. The larger it is, the more complex it is to review and the more likely it will be delayed. Remember that reviewing PRs is taking time from someone else's day.

Could you please review your PR and submit only the relevant code? Thank you!

@fmachado fmachado self-assigned this Sep 27, 2020
@BrentonPoke
Copy link
Author

BrentonPoke commented Sep 27, 2020

All the additional changes you're seeing are the result of the check style formatting script. It never would have made it through the Travis without it. I can try this again, but checkstyle has been causing issues.

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.

3 participants