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.

Sign up for GitHub

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

Jump to bottom

[Auth] Prefer immutable properties and private access level in *Request types #14001

Merged
merged 13 commits into from
Oct 30, 2024
Merged

[Auth] Prefer immutable properties and private access level in *Request types #14001

merged 13 commits into from
Oct 30, 2024

Conversation

ncooke3
Copy link
Member

@ncooke3 ncooke3 commented Oct 30, 2024 •
edited
Loading

This should be a no-op. Moving to private and lets where applicable. I'm working on moving these types to structs and thought this would be a good cleanup opportunity to minimize mutability.

#no-changelog

ncooke3 added 13 commits October 30, 2024 16:09
@ncooke3
[Auth] Update SecureTokenRequest to have only immutable properties
8df9724
@ncooke3
[Auth] Update WithdrawMFARequest to have only immutable properties
ac67174
@ncooke3
[Auth] Update StartMFASignInRequest to have only immutable properties
5d465b0
@ncooke3
[Auth] Update FinalizeMFASignInRequest to have only immutable properties
8ceae78
@ncooke3
[Auth] Update StartMFAEnrollmentRequest to have only immutable proper…
233b3c7 
…ties
@ncooke3
[Auth] Update FinalizeMFAEnrollmentRequest to have only immutable pro…
c920a6a 
…perties
@ncooke3
[Auth] Update VerifyPhoneNumberRequest to have only immutable properties
7ded4e2
@ncooke3
[Auth] Update VerifyClientRequest to have only immutable properties
ba922b2
@ncooke3
[Auth] Update CreateAuthURIRequest to have only immutable properties
b9db78b
@ncooke3
[Auth] Update GetOOBConfirmationCodeRequest to have only immutable pr…
70901ae 
…operties
@ncooke3
[Auth] Update RevokeTokenRequest to have only immutable properties
054ab16
@ncooke3
[Auth] Lock down property visibility for SetAccountInfoRequest
df79d27
@ncooke3
Various fixes
c070fc5
@ncooke3 ncooke3 requested a review from paulb777 October 30, 2024 21:28
@ncooke3 ncooke3 assigned andrewheard and unassigned andrewheard Oct 30, 2024
@ncooke3 ncooke3 requested a review from andrewheard October 30, 2024 21:30
paulb777
paulb777 approved these changes Oct 30, 2024
View reviewed changes
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

andrewheard
andrewheard approved these changes Oct 30, 2024
View reviewed changes
Copy link
Contributor

@andrewheard andrewheard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as well. The one benefit of leaving them as internal would be to avoid discouraging testing (it's easier with @testable import and internal) but I imagine most of us would just change the access level back if we needed to.

@ncooke3 ncooke3 merged commit 56b0475 into main Oct 30, 2024
55 checks passed
@ncooke3 ncooke3 deleted the nc/auth-swift6 branch October 30, 2024 22:33
@github-actions github-actions bot mentioned this pull request Nov 13, 2024
Merged
@firebase firebase locked and limited conversation to collaborators Nov 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@andrewheard andrewheard andrewheard approved these changes

@paulb777 paulb777 paulb777 approved these changes

Assignees
No one assigned
Labels
api: auth
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ncooke3 @paulb777 @andrewheard @google-oss-bot