-
Notifications
You must be signed in to change notification settings - Fork 102
Remove vestiges of CC upgrade by replacement #1542
Conversation
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.
Thanks! Just one q?
// Returns an error if the target sector cannot be found, or some other bad state is reached. | ||
// Returns false if the target sector is faulty, terminated, or unproven | ||
// Returns true otherwise | ||
func (st *State) CheckSectorHealthExcludeUnproven(store adt.Store, dlIdx, pIdx uint64, sector abi.SectorNumber) (bool, error) { | ||
func (st *State) CheckSectorHealthExcludeUnproven(store adt.Store, dlIdx, pIdx uint64, sector abi.SectorNumber, requireProven bool) (bool, error) { |
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 add this if it's never being called with false?
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.
So that someone doesn't copy-paste this method in the future when they want a minor change in functionality.
I can remove this flag if @ZenGround0 concurs.
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.
LGTM
// Returns an error if the target sector cannot be found, or some other bad state is reached. | ||
// Returns false if the target sector is faulty, terminated, or unproven | ||
// Returns true otherwise | ||
func (st *State) CheckSectorHealthExcludeUnproven(store adt.Store, dlIdx, pIdx uint64, sector abi.SectorNumber) (bool, error) { | ||
func (st *State) CheckSectorHealthExcludeUnproven(store adt.Store, dlIdx, pIdx uint64, sector abi.SectorNumber, requireProven bool) (bool, error) { |
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.
LGTM
Codecov Report
@@ Coverage Diff @@
## master #1542 +/- ##
========================================
+ Coverage 69.3% 69.7% +0.3%
========================================
Files 72 72
Lines 8737 8690 -47
========================================
- Hits 6060 6057 -3
+ Misses 1817 1774 -43
+ Partials 860 859 -1 |
No functional change, the removed code was unreachable.