Removed settings.Usernames and reactivate PreferredName #3964
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (69)
|
saledjenic
approved these changes
Aug 28, 2023
There was a problem hiding this comment.
I logged the issue to remove it from the desktop app implementation, here status-im/status-desktop#12017.
qfrank
force-pushed
the
feat/sync_ens_name
branch
from
15809f4
to
38c23f8
Compare
igor-sirotin
approved these changes
Aug 29, 2023
There was a problem hiding this comment.
Thanks for the update 👍
I also just digged a bit more into this topic and found a good approach for marking deleted fields:
https://protobuf.dev/programming-guides/proto3/#fieldreserved
Feel free to add it or not, I'm just sharing what I found 🙂
qfrank
force-pushed
the
feat/sync_ens_name
branch
3 times, most recently
from
6a10c26
to
512685d
Compare
qfrank
force-pushed
the
feat/sync_ens_name
branch
3 times, most recently
from
223d215
to
2b6f320
Compare
saledjenic
reviewed
Sep 14, 2023
| response := wakusync.WakuBackedUpDataResponse{ | ||
| Setting: settingField, | ||
| if settingField != nil { | ||
| if message.GetType() == protobuf.SyncSetting_PREFERRED_NAME && message.GetValueString() != "" { |
There was a problem hiding this comment.
@qfrank the changes look good, I have only 2 general questions:
- Do we need to display the ens name at all on the login screen or we display the display name and use the preferred ens name after login? (This is more about whether is that a request or not?)
- What in case the user changes the setting or removes the ens name but the backup still refers to the preferred ens name which is not in use anymore?
There was a problem hiding this comment.
Do we need to display the ens name at all on the login screen or we display the display name and use the preferred ens name after login? (This is more about whether is that a request or not?)
I had the same feeling, and @cammellos made a clarification here
What in case the user changes the setting or removes the ens name but the backup still refers to the preferred ens name which is not in use anymore?
No process in this case , mobile doesn't support remove ENS ATM @saledjenic
qfrank
force-pushed
the
feat/sync_ens_name
branch
5 times, most recently
from
1d0edc6
to
9247759
Compare
remove settings.usernames
qfrank
force-pushed
the
feat/sync_ens_name
branch
from
9247759
to
8cb11a3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this PR removed
settings.Usernamesand reactivate syncsettings.PreferredName, mobile will use the same API relate to ens as desktop now, e.g.Registerrelate mobile issue
relate mobile PR