refactoring and removing unused imports,some sonarlint fixes #126
Conversation
|
Things like this should usually be split up at least in different commits if not PRs (the latter being more of a project workflow preference). Having separate commits for things like refactoring, lint fixes, and removing unused code makes it possible to to run various checks a lot easier. It also makes it possible to use |
|
Thanks for the suggestion will keep that in mind. Should I create new PR for above changes which is split up in diff commits? |
|
Thatks up to the folks actually reviewing this and managing the code base, I'm just speaking from experience managing other projects over time. |
| @@ -63,14 +63,14 @@ public BaseAccountAuthenticator(AccountsManager accountsManager, Clock clock) { | |||
| AuthorizationHeader authorizationHeader = AuthorizationHeader.fromUserAndPassword(basicCredentials.getUsername(), basicCredentials.getPassword()); | |||
| Optional<Account> account = accountsManager.get(authorizationHeader.getIdentifier()); | |||
|
|
|||
| if (!account.isPresent()) { | |||
| if (isNull(account)) { | |||
There was a problem hiding this comment.
I don't think Objects.isNull checks whether or not Optional's are present or empty. I think this will just check if the Optional is null.
There was a problem hiding this comment.
It looks like this change was reverted in #127.
|
will delete this PR and raise other as per the suggestions |
|
For future reference, PRs are mutable. You shouldn't close one just to open another one for the same core submission, just remove the unrelated commit from the source branch. Also using your fork's |
Keeping this pull request small for ease of review. Can add create similar requests for sonarlint fixes and refactor in iterative manner in future.