1794207 - Rename FxaAccountManager.withConstellation to show that it can fail

Status: RESOLVED FIXED

(Firefox for Android :: Accounts and Sync, task)

Description

[Jonathan Almeida [:jonalmeida]]

From github: https://github.com/mozilla-mobile/android-components/issues/7139.

Based on the comment from this review: https://github.com/mozilla-mobile/android-components/pull/7114#discussion_r431357325

We should rename the extension function FxaAccountManager.withConstellation to be clear that this is a maybe-operation only if an OAuthAccount exists.

Possibly: withConstellationIfExists but lets talk about a better suggestion before putting up patches.

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

[Lalit Shankar Chowdhury]

Attachment: Bug 1794207 - Rename `FxaAccountManager.withConstellation` to show it can fail. r=jonalmeida

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.

Comment 3

[Thomas JOUANNOT [:mazerty]]

hi!
brand new contributor here, i'd like to take this bug if you don't mind :)
i'd like to have a basic understanding of the contributing workflow before suggesting a feature that i'd like to work on :)

i'm reading the documentation in the meantime, but i wouldn't mind some mentoring ;)

Comment 4

[Thomas JOUANNOT [:mazerty]]

Attachment: Bug 1794207 - Rename FxaAccountManager.withConstellation to show it can fail r=jonalmeida

Comment 5

[Thomas JOUANNOT [:mazerty]]

oh... now that i've read a bit about moz-phab i realized that maybe i could have just fetched the patch from the previous contributor and completed it with your review feedback, am i right ?
sorry if i made a mess by creating a new "pull request" ^^"

Comment 6

[Jonathan Almeida [:jonalmeida]]

Hi Thomas, I think what you've done is fine. Sorry, I haven't gotten to your patch yet - it overall looks good to me. I'll leave one recommendation in it in a second and also push it for a CI run to validate that nothing has broken.

Comment 7

[Thomas JOUANNOT [:mazerty]]

alright :)
just a quick question : how do i do that ?
i mean : do i just add a commit on top of mine and then run moz-phab again?
will it pick up everything ?

from what i understand moz-phab is basically just a patch/diff manager, and it looks at the commit on "main/master" to determine what needs to be included in the patch, but i'm wondering if i should but the same commit message in the second commit so that it can link everything together?

thanks for your help :)

Comment 8

[Thomas JOUANNOT [:mazerty]]

oh maybe i should mention that i'm still on mercurial for now

Comment 9

[Thomas JOUANNOT [:mazerty]]

hahaha nevermind
i tested moz-phab patch D258283 on a new "default" and it checked out my commit again
then i could just amend and moz-phab again and it worked :D

Comment 10

[Jonathan Almeida [:jonalmeida]]

Great! Yeah, that solution works too - if you still had your original patch, you can hg commit --amend it and moz-phab submit would update your existing patch.

You should try out the new mozilla-firefox/firefox git repo - it's quite nice and all our tools still work there. :)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/915c97592c6d
https://hg.mozilla.org/integration/autoland/rev/b63d5b703a2f
Rename FxaAccountManager.withConstellation to show it can fail r=jonalmeida,android-reviewers

Comment 12

[Thomas JOUANNOT [:mazerty]]

(In reply to Jonathan Almeida [:jonalmeida] from comment #10)

You should try out the new mozilla-firefox/firefox git repo - it's quite nice and all our tools still work there. :)

yeah! actually i came from https://github.com/mozilla-mobile/firefox-android which only mentions the migration from github to mercurial last year, that's why i checked out mercurial and struggled a bit ^^"
now that's it's merged, i'll restart from git yes

so yeah thank you for the merge :) it's nice that's the github commit linked my account there !

about my next step, i'd like to take that ticket : https://bugzilla.mozilla.org/show_bug.cgi?id=1967168
and i was wondering if you could help me a bit?
i'm a senior developer in general, but i only have a bit of experience with kotlin/android (my own minimal launcher with compose)
my questions would be mostly about the specifics of firefox (components, code structure) + in the case of that ticket, maybe some kind of specification about how to actually implement the feature (configuration key, ui design if any, stuff like that)

Comment 13

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/b63d5b703a2f

Comment 14

[Jonathan Almeida [:jonalmeida]]

I see you've made some progress in that issue, so I'll remove the NI here. :)

Feel free to hang out and say hi in our matrix channel: #fenix:mozilla.org