Gov cleanup + copyright bump#2324
Conversation
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
Nice :) Noticed a couple of places where some manual fixing/refactoring could be a good idea, see inline comments.
| << strDataAsPlainString | ||
| << std::endl; | ||
| ); | ||
| } std::cout |
| return true; | ||
| } | ||
| case GOVERNANCE_OBJECT_TRIGGER: { | ||
| if (!fCheckCollateral) |
| switch (nOutcome) { | ||
| case VOTE_OUTCOME_NONE: | ||
| return "NONE"; | ||
| break; |
There was a problem hiding this comment.
Interesting.. break;s make no sense here..
| break; | ||
| switch (nSignal) { | ||
| case VOTE_SIGNAL_NONE: | ||
| strReturn = "NONE"; |
There was a problem hiding this comment.
Sigh... Two similar functions, two different approaches... And both could actually be refactored/simplified to work the way ConvertVoteSignal does it.
| case MSG_GOVERNANCE_OBJECT_VOTE: | ||
| { | ||
| if(cmapVoteToObject.HasKey(inv.hash)) { | ||
| } break; |
There was a problem hiding this comment.
Let's move this break; inside (i.e. smth like s/} break;/break;\n }/).
| } | ||
| } | ||
| break; | ||
| } break; |
|
|
||
| if (fUpdateFailStatus) | ||
| it->second.fStatusOK = false; | ||
| if (fUpdateFailStatus) it->second.fStatusOK = false; |
There was a problem hiding this comment.
I'd rather add braces in original code here.
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
I wasn't able to find any place where enum->string conversion results were used besides some log messages. I think the reason for uppercase was to make them easily grep-able when debugging. In any case, this changes the functionality, so doesn't quite qualify for refactoring and we shouldn't mix the two. IMO should keep enum->string conversion (SmthToString()) results uppercase as they were, they could be adjusted in future PRs if needed (StringToSmth() input strings should be kept lowercase).
Also, found one unused function which can be dropped, see inline comments.
| void Relay(CConnman& connman) const; | ||
|
|
||
| std::string GetVoteString() const { | ||
| std::string GetVoteString() const |
There was a problem hiding this comment.
Looks like this function is not used anywhere at all. Can be dropped imo.
…rtOutcomeToString" This reverts commit 41bc3d3.
|
Yeah, I expected us not to want that commit in this pr. Dropped that function. Re-review |
2 participants
Clang formatting based, with a few manual changes