Details
Diff Detail
- Repository
- rCOMM Comm
- Branch
- master
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/types/minimally-encoded-thread-permissions-types.js | ||
|---|---|---|
| 189 | NOTE: It definitely doesn't make sense for ThreadInfo to live in minimally-encoded-permissions-types instead of lib/types-thread-types. I will move these over at the end of the stack. | |
| lib/types/thread-types.js | ||
| 66 | Snuck in removal of this as well. It was unused after removing legacyThreadInfoValidator and it made sense to remove instead of adding to exports imo. | |
| lib/permissions/minimally-encoded-thread-permissions-validators.js | ||
|---|---|---|
| 15–16 | Can you revert import ordering changes before landing? | |
| 58 | Should this be renamed to threadInfoValidator for consistency with the ThreadInfo type? Or is that done in a later diff? | |
| lib/types/minimally-encoded-thread-permissions-types.js | ||
| 189 | Thanks for noting this | |
| web/types/nav-types.js | ||
| 49 | A bit confused that this wasn't a union before... wouldn't it be causing issues if it only worked with LegacyThreadInfos? | |
| web/types/nav-types.js | ||
|---|---|---|
| 49 | Did a little digging and it looks like navInfoValidator is only used as part of initialReduxStateValidator which is only used to validate response from getInitialReduxStateResponder. pendingThread is set based for the following condition:  and it would be set to a ThreadInfo (minimallyEncoded):  and so in that scenario I do think there would be issues. This is something I should've caught earlier. | |
| lib/permissions/minimally-encoded-thread-permissions-validators.js | ||
|---|---|---|
| 58 | Handled in later diff. | |