Page MenuHomePhabricator

[backup-service][backup-client] rename `username` -> `user_identifier`
ClosedPublic

Authored by kamil on Tue, Oct 29, 9:19 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 16, 3:12 AM
Unknown Object (File)
Fri, Nov 15, 9:38 PM
Unknown Object (File)
Fri, Nov 15, 6:35 AM
Unknown Object (File)
Thu, Nov 14, 3:48 AM
Unknown Object (File)
Sun, Nov 10, 3:33 PM
Unknown Object (File)
Sun, Nov 10, 12:47 AM
Unknown Object (File)
Sat, Nov 9, 7:14 PM
Unknown Object (File)
Sat, Nov 9, 6:48 AM
Subscribers

Details

Summary

ENG-6145.

username is consfusing, as it could be the Wallet address too

Depends on D13807

Test Plan

run commtest

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
native/native_rust_library/src/backup.rs
328 ↗(On Diff #45430)

this is going to be updated later as part of Native changes for minimal version of backup

kamil published this revision for review.Wed, Oct 30, 9:29 AM
varun requested changes to this revision.Fri, Nov 1, 2:17 PM
varun added inline comments.
native/native_rust_library/src/backup.rs
334 ↗(On Diff #45457)

confused why we use user_id here

This revision now requires changes to proceed.Fri, Nov 1, 2:17 PM
kamil requested review of this revision.Sun, Nov 3, 11:25 PM
kamil added inline comments.
native/native_rust_library/src/backup.rs
334 ↗(On Diff #45457)

We don't have a user_identifier (username/wallet) here. The initial version of the backup used user_id - and the point of this whole stack is to change this, this diff is only about renaming service and client prop.

Later in the stack in D13837 I implemented code that uses user_identifier correctly, in D13849 I updated rest of the code to start using this and this particular method is removed.

I know this is confusing, but there was not better option to slice it for diffs, I could either cause CI falling or submit one diff with squashed all commits form this stack.

This revision is now accepted and ready to land.Mon, Nov 4, 7:31 AM