Page MenuHomePhabricator

[lib] Update KeyserverCall type
AbandonedPublic

Authored by inka on Sep 19 2023, 7:19 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Sep 29, 3:39 AM
Unknown Object (File)
Sun, Sep 29, 3:39 AM
Unknown Object (File)
Fri, Sep 6, 3:50 PM
Unknown Object (File)
Fri, Sep 6, 3:40 PM
Unknown Object (File)
Aug 30 2024, 2:46 PM
Unknown Object (File)
Aug 28 2024, 7:23 AM
Unknown Object (File)
Aug 26 2024, 6:16 AM
Unknown Object (File)
Aug 25 2024, 11:27 PM
Subscribers

Details

Reviewers
michal
kamil
ginsu
Summary

issue: https://linear.app/comm/issue/ENG-4681/create-action-objects-for-all-actions
Some actions take multiple parameters. I think it would be better if actions always took a single input argument that would contain all the data required in its fields. This will let us avoid having // eslint-disable-next-line no-unused-vars in many places in keyserverIDExtractor function definitions (since we want keyserverIDExtractor to take the same set of arguments as the action is it created for). This will also fix the one thing I don't like about how CallKeyserverEndpoint is typed - that it takes an array of arguments passed into the action. So instead of callServerEndpoint('search_messages', request, [arg1, arg2]); we would be able to just call callServerEndpoint('search_messages', request, input);. This will simplify the code

Test Plan

Ran yarn flow-all

Diff Detail

Repository
rCOMM Comm
Branch
inka/actions
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka requested review of this revision.Sep 19 2023, 7:37 AM

Nice!

lib/utils/keyserver-call.js
155

I think this is always true? And it's a bit more strictly typed

This revision is now accepted and ready to land.Sep 20 2023, 3:28 AM
lib/utils/keyserver-call.js
155

Well, there are actions that take only one argument that is an array for example, and I don't think it makes sense to be adding boilerplate code and wrapping them in objects, so I leave them as they are

After changes to D9217 this diff is no longer needed