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

android: use EditPrefs instead of passing UpdatePrefs to start #370

Merged
merged 1 commit into from
May 7, 2024

Conversation

oxtoacart
Copy link
Contributor

This allows us to precisely set the options we need during login and avoid wiping away defaults like AllowSingleHost.

Updats tailscale/tailscale#11731

@oxtoacart oxtoacart requested review from bradfitz and kari-ts and removed request for bradfitz and kari-ts May 7, 2024 03:52
@MulverineX
Copy link

"Updats", whoops! 😝

fun login(options: Ipn.Options = Ipn.Options(), completionHandler: (Result<Unit>) -> Unit = {}) {

fun login(
options: Ipn.Options = Ipn.Options(),
Copy link
Member

Choose a reason for hiding this comment

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

maybe you should just take the authKey string only? You don't have separate frontend-vs-backend log IDs on Android now, do you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call.

start.onFailure { completionHandler(Result.failure(it)) }.onSuccess { loginAction() }
val startAction = {
Client(viewModelScope).start(options) { start ->
start.onFailure { completionHandler(Result.failure(it)) }.onSuccess { loginAction() }
Copy link
Member

Choose a reason for hiding this comment

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

if this is the style, that's fine, but I personally think it's sad that onFailure and onSuccess's bodies don't line up in any way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's the auto-formatter.

This allows us to precisely set the options we need during login
and avoid wiping away defaults like AllowSingleHosts.

Updates tailscale/tailscale#11731

Signed-off-by: Percy Wegmann <percy@tailscale.com>
@oxtoacart oxtoacart merged commit 31b0ec8 into main May 7, 2024
4 checks passed
@oxtoacart oxtoacart deleted the percy/11731 branch May 7, 2024 15:45
@oxtoacart
Copy link
Contributor Author

Thanks for the review @agottardo, @bradfitz and @MulverineX

kari-ts added a commit to tailscale/tailscale that referenced this pull request May 7, 2024
Web version of tailscale/tailscale-android#370
This allows us to update the prefs rather than creating new prefs

Updates #11731

Signed-off-by: kari-ts <kari@tailscale.com>
kari-ts added a commit to tailscale/tailscale that referenced this pull request May 7, 2024
Web version of tailscale/tailscale-android#370
This allows us to update the prefs rather than creating new prefs

Updates #11731

Signed-off-by: kari-ts <kari@tailscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants