-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Cleanup #470
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
Cleanup #470
Conversation
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
@NonNull | ||
private final GoogleApiClientTaskHelper mClientHelper; | ||
|
||
private CredentialsApiHelper(GoogleApiClientTaskHelper gacHelper) { | ||
mClientHelper = gacHelper; | ||
} | ||
|
||
public CredentialRequest createCredentialRequest(List<IdpConfig> providers) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only non move stuff around change: a few of these methods weren't being used so I removed them.
Signed-off-by: Alex Saveau <[email protected]>
# Conflicts: # auth/src/main/java/com/firebase/ui/auth/ui/email/CheckEmailFragment.java
# Conflicts: # auth/src/main/java/com/firebase/ui/auth/ui/email/fieldvalidators/RequiredFieldValidator.java
} else { | ||
mListener.onExistingIdpUser(new User.Builder(email) | ||
.setProvider(providers.get(0)) | ||
.addOnCompleteListener(getActivity(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the formatting between this complete listener and the success listener that follows is inconsistent. I prefer any style that minimizes indentation which means I vote for either the style before this change or the success listener style below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
} | ||
} | ||
|
||
private void showEmailAutoCompleteHint() { | ||
try { | ||
startIntentSenderForResult(getEmailHintIntent().getIntentSender(), RC_HINT, null, 0, 0, 0, null); | ||
startIntentSenderForResult(getEmailHintIntent().getIntentSender(), | ||
RC_HINT, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we don't do this (argument alignment) throughout the project, any reason to start with this method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, we should have been using mFragmentHelper.startIntentSenderForResult
anyway to get rid of all those zeros and nulls.
Just a comment or two about style (which I only bothered to do since this PR is mostly about style in the first place) |
Signed-off-by: Alex Saveau <[email protected]>
This is an optional-ish PR (not that my other PRs were required! 😄) that has been waiting for #447.
I've noticed that most people (including @samtstern in his email refactor) put their static factory methods at the top of classes and Intellij keeps reformatting my code in that way so I thought I'd create a PR to fix that.
This PR looks big, but it's mostly just moving stuff around and letting Intellij do its thing.