add power table cli #3379
add power table cli #3379
Conversation
deaswang
force-pushed
the
feat/3028-power-table
branch
from
e68ec6a
to
6ce3ab6
Compare
deaswang
force-pushed
the
feat/3028-power-table
branch
from
6ce3ab6
to
3818c43
Compare
There was a problem hiding this comment.
@ZenGround0 you wrote the issue calling for this, could you please take a look when you get a chance?
| @@ -47,7 +49,8 @@ var actorCmd = &cmds.Command{ | |||
| Tagline: "Interact with actors. Actors are built-in smart contracts.", | |||
| }, | |||
| Subcommands: map[string]*cmds.Command{ | |||
| "ls": actorLsCmd, | |||
| "ls": actorLsCmd, | |||
| "power-table": actorPowerCmd, | |||
There was a problem hiding this comment.
Only miner actors can have power, and it would be more findable if this command were with the existing miner power command. Please move this to the miner command. Move the tests appropriately.
| node := b.WithGenesisInit(testGen). | ||
| WithConfig(func(c *config.Config) { | ||
| c.Mining.AutoSealIntervalSeconds = 120 | ||
| }). |
| out := cmd.RunSuccess(ctx, "actor", "power-table").ReadStdout() | ||
| assert.Contains(t, out, minerAddr1.String()) | ||
| assert.Contains(t, out, minerAddr2.String()) | ||
| assert.Contains(t, out, minerAddr3.String()) |
There was a problem hiding this comment.
Please assert the entire output. I hope that the order is deterministic.
|
I am on the fence about accepting this. See my comment #3028 (comment) The one reason I'd accept it is in recognition that @deaswang you stepped up to take on an issue that we had filed (although categorized as icebox) and we really value your engagement. Nevertheless, I think this code will be deleted as we migrate to a new API in the future. |
|
I'll echo @anorth in saying that your work is high quality and valuable for the project @deaswang. From the above it sounds like the right call is to close this down and close the issue too. @anorth, @deaswang how do you think we can prevent mixups like this in the future. Should we ask contributors to comment below issues they want to take on so that we can catch things we've subsequently decided against before contributors put in work? Should we be asking contributors to only consider candidate issues (might require zenhub which contributors can't access) or issues with a certain label, i.e. |
|
I do think contributors should post a comment on issues they are intending to work on, and be then assigned to such issue to indicate that status. ZenHub is accessible to all contributors: it's free for open source projects. It's documented and linked from our contributing guide. I would expect regular contributors to consult it. |
fixes #3028
use cli
go-filecoin actor power-table.