Repository navigation
Conversation
|
coverage: 82.16% (+0.1%) from 82.02% — MayRosenbaum:json_manipulation_automation_skeleton into hyperledger:main |
| party := modify.Command("party", "Add or remove ARMA parties, or change a party's nodes and CA lists.") | ||
|
|
||
| add := party.Command("add", "Add a new ARMA party and its orderer organization, if it does not already exist.") | ||
| partyDef := add.Flag(flagParty, "Path to the party definition YAML.").Required().ExistingFile() |
There was a problem hiding this comment.
The --party flag has inconsistent meaning across sibling subcommands. Here (modify party add) --party is a YAML file path (.ExistingFile()), but in modify party node and modify party ca (lines 364, 398) the same --party flag is a numeric PartyID (.Uint32()). Likewise --org is a YAML path in modify app add but an org name in modify app known-certs. Overloading one flag name with two semantics is error-prone; use distinct flag names (e.g. --party-def vs --party-id).
There was a problem hiding this comment.
I think its better to keep it as it is
|
|
||
| remove := party.Command("remove", | ||
| "Remove an ARMA party (and its orderer org, unless another party is still associated with it).") | ||
| partyID := remove.Arg("party-id", "Numeric PartyID to remove.").Required().Uint32() |
There was a problem hiding this comment.
Inconsistent invocation style for the same identifier across sibling subcommands. party remove takes the PartyID as a positional Arg, while party node/party ca take it as the --party flag; similarly app remove takes the org name as a positional Arg while app known-certs add|remove take --org. Pick one convention (positional or flag) for the target identifier so the command surface is predictable.
There was a problem hiding this comment.
this follows the desing in the readme, lets keep it
3f75af3 to
86c8df8
Compare
Signed-off-by: May.Buzaglo <May.Buzaglo@ibm.com>
Signed-off-by: May.Buzaglo <May.Buzaglo@ibm.com>
86c8df8 to
47b0ecf
Compare
3 participants
Type of change
Description
JSON manipulation automation in fxadmin - skeleton
Related issues
issue #187