Skip to content
This repository has been archived by the owner on Jun 6, 2023. It is now read-only.

Remove vestiges of CC upgrade by replacement #1542

Merged
merged 2 commits into from
Dec 20, 2021
Merged

Conversation

anorth
Copy link
Member

@anorth anorth commented Dec 14, 2021

No functional change, the removed code was unreachable.

@anorth anorth requested a review from ZenGround0 December 14, 2021 22:32
@anorth anorth requested a review from a team as a code owner December 14, 2021 22:32
Copy link
Collaborator

@arajasek arajasek left a 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) {
Copy link
Collaborator

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?

Copy link
Member Author

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.

Copy link
Contributor

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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov-commenter
Copy link

Codecov Report

Merging #1542 (09c7d61) into master (97a6ac1) will increase coverage by 0.3%.
The diff coverage is 40.0%.

@@           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     

@ZenGround0 ZenGround0 merged commit 8d7c052 into master Dec 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants