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

Fixes issue: InfluxDBResultMapper doesnt call POJO setter #676

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sjoshid
Copy link

@sjoshid sjoshid commented May 9, 2020

Closes #670

Things included in this PR:

  • Call field setters instead of setting field values directly.
  • This PR maintains existing behaviour ie setting fields directly ignoring setters.
  • Priority is given to field setter if one exists. If not, field is set directly.

1) Calling field setters if available before falling back to setting fields directly.
@sjoshid
Copy link
Author

sjoshid commented May 9, 2020

@fmachado
Im not sure if I need to run format sh. But when I do, I get a lot of diffs in most Java files.
Anything I need to do with formattting?

@fmachado
Copy link
Contributor

@sjoshid thanks for submitting this PR.

Im not sure if I need to run format sh.

I don't remember when was the last time that I had to run that shell script as I'm using the checkstyle.xml for a while.

But before getting into the formatting issues, could you please check why Travis build failed?

https://travis-ci.org/github/influxdata/influxdb-java/builds/685113586

sjoshid added 2 commits May 10, 2020 15:57
1) Handling NPE
1) Fixing Checkstyle errors.
@codecov-io
Copy link

codecov-io commented May 10, 2020

Codecov Report

Merging #676 into master will decrease coverage by 0.06%.
The diff coverage is 88.17%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #676      +/-   ##
============================================
- Coverage     88.18%   88.11%   -0.07%     
- Complexity      722      737      +15     
============================================
  Files            69       69              
  Lines          2513     2566      +53     
  Branches        268      285      +17     
============================================
+ Hits           2216     2261      +45     
- Misses          208      210       +2     
- Partials         89       95       +6     
Impacted Files Coverage Δ Complexity Δ
...n/java/org/influxdb/impl/InfluxDBResultMapper.java 87.75% <88.17%> (-1.06%) 72.00 <28.00> (+15.00) ⬇️

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 ad5d6a8...5c9bd27. Read the comment docs.

@sjoshid sjoshid changed the title PR for #670 Fixes issue: InfluxDBResultMapper doesnt call POJO setter May 11, 2020
@sjoshid
Copy link
Author

sjoshid commented May 21, 2020

@fmachado
Any feedback on this one?

@sjoshid
Copy link
Author

sjoshid commented Sep 29, 2020

@majst01
I have been waiting for @fmachado for feedback but havent received it.
Can you provide one?

@John9570
Copy link

whats the status on this? this would be a super useful feature to have

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InfluxDBResultMapper doesnt call POJO setter
4 participants