Fix skip confirm logic #7085
Fix skip confirm logic #7085
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7085 +/- ##
==========================================
- Coverage 55.60% 54.54% -1.06%
==========================================
Files 457 550 +93
Lines 27440 37690 +10250
==========================================
+ Hits 15257 20558 +5301
- Misses 11083 15442 +4359
- Partials 1100 1690 +590 |
|
Liveness test is broken on master and being tackled here #7095 |
sahith-narahari
requested review from
alessio,
alexanderbez,
clevinson,
fedekunze and
jgimeno
as code owners
| if key == nil { | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
if the pubkey is nil, error makes sense. Is there a reason to handle nil separately?
There was a problem hiding this comment.
nil is a valid pubkey, so it shouldn't error
There was a problem hiding this comment.
I thought PublicKey was a sum. Can you explain how nil is a valid pubkey?
Edit: we're migrating to Any, but imo it should still not be nil?
There was a problem hiding this comment.
I don't have full context, @aaronc can explain better
There was a problem hiding this comment.
All fields in proto are optional by default but pubkey is definitely optional. nil is correct here. I can't think of any other good way to handle this. Let's add an explanatory comment.
There was a problem hiding this comment.
We should also add a case for key.Sum being nil.
amaury1093
added
the
A:automerge
Description
This PR addresses issues with flags skip-confirmation and dry-run
closes: #7079
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes