Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move PolyBFT to Edge #774
Move PolyBFT to Edge #774
Changes from all commits
9f01ed5
483d7ed
ed4c4d7
f5bac88
119ef2f
6ef6667
2c466fd
362aee9
b14cf17
48b179c
658f821
4556e09
39a33f0
d69b908
d4f56d4
44713ef
7bc8713
168bf82
d68816c
2e8da0c
7571d55
85f85ff
61f25a7
127e455
2bf3ff7
09ec57b
3869e8e
6cb583e
c6771b2
2b4f677
a303343
84742fc
6b57a8a
19e15ca
824cce3
66ca172
f5d9103
b2fd576
1627b0c
86bf55d
96e4f56
f624c48
6463ce7
7dcd40f
49eb270
28eb649
053f803
07ab165
4bcf151
2ec8b89
6c838a9
07faccf
44ad7bc
d556755
b08f651
bfd4148
3999dda
394d624
c43fca5
76ba046
d9a3658
7c0f2cf
dc0bb8c
810272b
11624aa
7f2f631
02d2457
cd0629c
7d658e5
74176e0
67bf0d2
ad77bc8
cdc0bda
c8b4ad7
d2201b4
f8a27cc
7de4499
1712358
e926ca4
209fd4f
3c8182b
eff10fd
9ed8b3b
6ce2cae
7515a88
b79ddbd
654491a
f1b0b3c
62a4f8c
b54d4ec
989da00
00883a1
6047c44
3e87d3e
677d716
224744f
604c2ff
e27dca1
7e0e098
7998164
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this leftover?
Remove it if it's unused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this moved to
runPreRun
?Please move it back, as all methods that have required flags initialize them in the standard
GetCommand
methodThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the correct place in init flow, in this case, different required flags are required depending on the selected consensus, at this point of the execution
if p.isIBFTConsensus()
orp.consensusRaw
is set to its default value.Conditionally required flags should be setup in the
PreRunE
, see for example spf13/cobra#1595 (comment)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not extract this second part of the conditional into a separate method like
hasEpochs
, or something similar?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not usually a fan of moving oneliners to 4 lines methods, specially if they are only used once.