multi: Rename vote choice IsIgnore to IsAbstain. #656
Conversation
chaincfg/init.go
Outdated
| @@ -60,7 +60,7 @@ func shift(mask uint16) uint { | |||
|
|
|||
| func validateChoices(mask uint16, choices []Choice) error { | |||
| var ( | |||
| isIgnore, isNo int | |||
| isAbstain, isNo int | |||
There was a problem hiding this comment.
unrelated, but why are these ints?
There was a problem hiding this comment.
They're counts (obviously poorly named ones). I just went through and did a find/replace.
EDIT: I can update them to better names though since this PR is already doing that.
There was a problem hiding this comment.
sounds good, thought they were bools.
There was a problem hiding this comment.
Renamed to numAbstain and numNo to better reflect the fact they're counts.
| @@ -121,21 +121,21 @@ var ( | |||
| Id: "Abstain", | |||
| Description: "Abstain voting for Pedro", | |||
| Bits: 0x0, // 0b0000 | |||
| IsIgnore: false, // XXX this is the invalid bit | |||
| IsAbstain: false, // XXX this is the invalid bit | |||
There was a problem hiding this comment.
It's a test to ensure a vote that has the vote bits set to 0, but is not properly marked as an abstaining vote causes an error.
There was a problem hiding this comment.
Now i'm really confused, I can understand that some invalid bit pattern would be an error and be interpreted as abstaining, but why is bits=0 an error?
There was a problem hiding this comment.
The choice has the Bits field set to 0 however the IsAbstain flag set to false. That is an invalid combination since a choice that doesn't have any bits set must be abstain.
There was a problem hiding this comment.
I prefer IsIgnore but meh if you guys feel strong about it
There was a problem hiding this comment.
oic, these are invalid choices. ok.
| IsNo: false, | ||
| }, | ||
| { | ||
| Id: "Yes", | ||
| Description: "Vote for Pedro", | ||
| Bits: 0x2, // 0b0010 | ||
| IsIgnore: true, // XXX this is the invalid choice | ||
| IsAbstain: true, // XXX this is the invalid choice |
There was a problem hiding this comment.
In this case, the Bits field is not 0, so having the IsAbstain flag set to true is an error. @marcopeereboom can correct me if I'm wrong, but I believe the intent here is to ensure the choices are consistently defined across agendas. That is to say that having no bits set for a particular agenda is always the abstaining choice.
There was a problem hiding this comment.
This reads more like setting 0x2 as the valid option to indicate abstaining. Don't know why that would ever be used.
| IsNo: false, | ||
| }, | ||
| { | ||
| Id: "Yes", | ||
| Description: "Vote for Pedro", | ||
| Bits: 0x2, // 0b0010 | ||
| IsIgnore: true, // XXX this is the invalid choice | ||
| IsAbstain: true, // XXX this is the invalid choice |
davecgh
force-pushed
the
isignore_to_isabstain
branch
from
31d7893
to
470778a
Compare
|
I included a major bump in the RPC server version to account for the breaking change. |
davecgh
force-pushed
the
isignore_to_isabstain
branch
from
470778a
to
cb0c2e5
Compare
This renames the chaincfg parameter for the vote choice which represents an abstaining vote to be named IsAbstain instead of IsIgnore since that more accurately describes its intent and behavior. It also updates the RPC server choice result field for isignore to be named isabstain to match and bumps the major version accordingly. Finally, it renames other internal variables which make use of the choice to include the word abstain as well for clarity and renames a couple of other internal variables.
davecgh
force-pushed
the
isignore_to_isabstain
branch
from
cb0c2e5
to
05d8456
Compare
This renames the
chaincfgparameter for the vote choice which represents an abstaining vote to be namedIsAbstaininstead ofIsIgnoresince that more accurately describes its intent and behavior.It also updates the RPC server choice result field for
isignoreto be namedisabstainto match.Finally, it renames other internal variables which make use of the choice to include the word abstain as well for clarity.
Closes #655.