generalize ec cmd to gov any committee
#8385
Conversation
turadg
force-pushed
the
8384-gov-cmd
branch
from
d14b237 to
fbcbd1d
Compare
| * instanceName?: string, | ||
| * }} detail | ||
| * @param {Awaited<ReturnType<makeRpcUtils>>} [optUtils] | ||
| */ | ||
| const processOffer = async function ({ toOffer, sendFrom }, optUtils) { | ||
| const processOffer = async function ( |
There was a problem hiding this comment.
can these use arrow function style as well?
There was a problem hiding this comment.
i believe Commander.js relies upon the this binding.
| cmd | ||
| .command('committee') | ||
| .description('accept invitation to join a committee') | ||
| .requiredOption( |
There was a problem hiding this comment.
requiredOptions can have a default value? bizarre.
There was a problem hiding this comment.
I did it that way in case someone adds the option and as an invalid value.
| const votingRight = cont.find( | ||
| it => it.instance === agoricNames.instance.economicCommittee, | ||
| ); | ||
| const votingRight = cont.find(it => it.instanceName === opts.instance); |
There was a problem hiding this comment.
I'm doing type-matching by eye, and it.instanceName doesn't look like the same kind of thing as opts.instance. Is this correct? I couldn't find an actual type declaration, nor any other use of opts.instance.
There was a problem hiding this comment.
opts comes from the CLI options so it's this up above,
.requiredOption(
'--instance <string>',
'Committee name under agoricNames.instances',
String,
'economicCommittee',
)That does match the instanceName out of the result set.
|
@turadg - This PR appears to be stuck. It's had a merge label for > 24 hours |
turadg
force-pushed
the
8384-gov-cmd
branch
from
1c78c7f to
cc61e93
Compare
|
@Mergifyio requeue |
✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically |
closes: #8384
Description
Generalizes the
eccommand to agovcommand. To maintain backwards compatibility it aliases that back toecand makes the default committee names that of the EC.It also includes some code cleanup like shape validation and keyring options.
Security Considerations
n/a
Scaling Considerations
n/a
Documentation Considerations
Backwards compatible. Usage for the command comes from the CLI help.
Testing Considerations
We really should have a test. It can be exercised in #8388 (we should update that PR to use this version and re-run those tests to confirm it still works)
Upgrade Considerations
n/a