Page MenuHomePhabricator

[native] Redux migration to patch in `specialRole` field
ClosedPublic

Authored by atul on Feb 15 2024, 1:50 PM.
Tags
None
Referenced Files
F1696757: D11099.id37660.diff
Fri, May 3, 2:58 PM
Unknown Object (File)
Thu, Apr 25, 12:49 PM
Unknown Object (File)
Thu, Apr 25, 12:48 PM
Unknown Object (File)
Mar 30 2024, 1:49 PM
Unknown Object (File)
Mar 8 2024, 3:26 AM
Unknown Object (File)
Feb 21 2024, 12:04 PM
Unknown Object (File)
Feb 19 2024, 1:32 PM
Unknown Object (File)
Feb 19 2024, 9:57 AM
Subscribers

Details

Summary
NOTE: Marked as DRAFT to indicate that this diff needs to be tested more thoroughly before landing and is "subject to change." This diff implicitly serves as the Test Plan for D11095 and D11097 which is another reason I'm putting it up in DRAFT state.

Depends on D11097

Test Plan

Will set breakpoints and observe that the shape of data is as expected at each step of the migration.

Will also inspect SQLite DB directly before/after to make sure that things are encoded as expected.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

NOTE: I do not believe that there is a need for a corresponding web migration based on discussion in D10826. Instead we can ask staff to log out/log back in. I can still introduce migration if asking staff to log out/log back in on web is getting disruptive. (cc @kamil, @ashoat)

EDIT: Will go ahead and write the migration. The previous time all staff had logged out/back in for another reason as a coincidence so it wasn't necessary, but migration will be less disruptive than asking staff to re-auth.

atul requested review of this revision.Feb 15 2024, 2:06 PM

What is the downside of making this diff a draft instead of marking its title with a label?

What is the downside of making this diff a draft instead of marking its title with a label?

Would that still make the diff viewable to reviewers?

Was mostly to show a usage of D11095 and D11097 even though testing wasn't complete. Will avoid [DRAFT] in commit name in future and write something more specific in Summary instead.

atul retitled this revision from [DRAFT][native] Redux migration to patch in `specialRole` field to [native] Redux migration to patch in `specialRole` field.Feb 22 2024, 7:54 PM
In D11099#321944, @atul wrote:

What is the downside of making this diff a draft instead of marking its title with a label?

Would that still make the diff viewable to reviewers?

Yes, a draft is visible and can be linked and accessed just like a normal diff. The only difference is that it doesn't appear in any queue.

Will avoid [DRAFT] in commit name in future and write something more specific in Summary instead.

I think it is OK to have a diff that shows e.g. the usage. My concern was about the fact that the diff was appearing in a couple of queues without any need - it wasn't actionable.

tomek added inline comments.
native/redux/persist.js
1158 ↗(On Diff #37284)

Shouldn't this version be bumped?

This revision is now accepted and ready to land.Feb 27 2024, 12:43 AM
native/redux/persist.js
1158 ↗(On Diff #37284)

It'll be bumped along with web migration, codeVersion bump, and keyserver deploy

Safe to land, persist.version will get bumped when flip the switch lands.

This revision was landed with ongoing or failed builds.Mar 20 2024, 11:51 AM
This revision was automatically updated to reflect the committed changes.
This revision is now accepted and ready to land.Mon, Apr 22, 10:42 AM
This revision was landed with ongoing or failed builds.Mon, Apr 22, 12:33 PM
This revision was automatically updated to reflect the committed changes.