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

Make token getters not return null, but empty optional #14

Merged
merged 4 commits into from
Apr 4, 2019
Merged

Make token getters not return null, but empty optional #14

merged 4 commits into from
Apr 4, 2019

Conversation

crazed-developer
Copy link
Contributor

I feel that returning null instead of an empty optional is counter intuitive.

At it makes it necessary to actually do two checks:

  1. Check if the optional is null
  2. Check if the optional is empty

I hope this request will be positively received

I feel that returning null instead of an empty optional is counter intuitive. 

At it makes it necessary to actually do two checks:
1. Check if the optional is null
2. Check if the optional is empty
Sorry, should have tested
This in relation to my first pull request
It can not be generally assumed that the login name is identical to the preferred username. On a newly created production user, the preferred username is the first name + four digits. Where as I log in with the email address. So This check is not valid in my opinion.
@codecov-io
Copy link

Codecov Report

Merging #14 into master will decrease coverage by 0.31%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #14      +/-   ##
=========================================
- Coverage     80.31%   80%   -0.32%     
- Complexity       85    86       +1     
=========================================
  Files            12    12              
  Lines           386   390       +4     
  Branches         28    28              
=========================================
+ Hits            310   312       +2     
- Misses           56    57       +1     
- Partials         20    21       +1
Impacted Files Coverage Δ Complexity Δ
...ay/api/client/auth/oauth2/model/OAuthResponse.java 69.23% <66.66%> (-3.5%) 7 <3> (+1)

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 a31608f...28ff365. Read the comment docs.

@sengopal
Copy link
Contributor

sengopal commented Apr 4, 2019

Thanks for the submission. LGTM. Will be made available in the next release version.

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