Page MenuHomePhabricator

[lib] Deprecate `minimallyEncodeRawThreadInfo`
ClosedPublic

Authored by atul on Jul 11 2024, 1:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 20, 9:47 AM
Unknown Object (File)
Wed, Nov 20, 9:47 AM
Unknown Object (File)
Wed, Nov 20, 9:47 AM
Unknown Object (File)
Fri, Nov 8, 11:05 PM
Unknown Object (File)
Fri, Nov 8, 3:47 PM
Unknown Object (File)
Fri, Nov 8, 12:25 PM
Unknown Object (File)
Fri, Nov 8, 12:25 PM
Unknown Object (File)
Fri, Nov 8, 12:51 AM
Subscribers

Details

Summary

We want to freeze existing logic for use in migration and rawThreadInfoFromServerThreadInfo while making it clear that it shouldn't be used in the future.


Depends on D12597

Test Plan

Just a rename.

Diff Detail

Repository
rCOMM Comm
Branch
july9 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 11 2024, 1:40 PM
Harbormaster failed remote builds in B30294: Diff 42242!
atul requested review of this revision.Jul 11 2024, 2:16 PM
ashoat requested changes to this revision.Jul 11 2024, 5:20 PM

Just two questions:

We want to freeze existing logic for use in migration and rawThreadInfoFromServerThreadInfo while making it clear that it shouldn't be used in the future.

First: is rawThreadInfoFromServerThreadInfo just for the shimming logic that handles old clients?

Second: it looks like there's a Flow error here that doesn't show up in the parent diff. Any context?

This revision now requires changes to proceed.Jul 11 2024, 5:20 PM

First: is rawThreadInfoFromServerThreadInfo just for the shimming logic that handles old clients?

Yes, so we can construct RawThreadInfo with member permissions for clients that still expect them. We will construct "new" RawThreadInfos by stripping out member permissions using same utility function that'll be used in client migrations.

Second: it looks like there's a Flow error here that doesn't show up in the parent diff. Any context?

Let me check

atul requested review of this revision.Jul 15 2024, 11:50 AM

Second: it looks like there's a Flow error here that doesn't show up in the parent diff. Any context?

The one flow issue I'm seeing is the following:

f33e9f.png (1×2 px, 1001 KB)

which is resolved in D12755

ashoat requested changes to this revision.Jul 17 2024, 12:03 PM

Actually I must've gotten confused about the CI. I'm actually not seeing any Flow issues, but there do appear to be ESLint issues.

Can you get this green before you pass back to me?

This revision now requires changes to proceed.Jul 17 2024, 12:03 PM

update to address ESLint and failing validator test. flow issue(s) still expected.

This revision is now accepted and ready to land.Jul 18 2024, 10:18 AM

This is purely rename diff, so safe to cherry-pick and land out of order.

This revision was landed with ongoing or failed builds.Jul 18 2024, 12:21 PM
This revision was automatically updated to reflect the committed changes.