Skip to content
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

Update check tip util functions to return both at-tip status and block id #294

Merged
merged 3 commits into from
Jan 12, 2021

Conversation

septerr
Copy link
Contributor

@septerr septerr commented Jan 12, 2021

Fixes # .

Motivation

We added CheckNetworkTip and CheckStorageTip utility functions to DRY these checks in the Rosetta CLI. These methods return a block identifier if the network or storage block is at tip respectively.

In some contexts, the CLI needs to know the block identifier even if it is not at tip.

Solution

Updated these methods to return at-tip status in addition to the network or storage block identifier.

Open questions

@coveralls
Copy link

coveralls commented Jan 12, 2021

Pull Request Test Coverage Report for Build 14242

  • 12 of 15 (80.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.01%) to 77.922%

Changes Missing Coverage Covered Lines Changed/Added Lines %
utils/utils.go 12 15 80.0%
Files with Coverage Reduction New Missed Lines %
utils/utils.go 1 66.54%
Totals Coverage Status
Change from base Build 14219: -0.01%
Covered Lines: 8287
Relevant Lines: 10635

💛 - Coveralls

utils/utils.go Outdated
if fetchErr != nil {
return nil, fmt.Errorf("%w: unable to fetch network status", fetchErr)
return false,
currentStorageBlock.BlockIdentifier,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: return nil here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was on the fence about this. We could leave it to the caller to use or ignore the identifier based on the error.
But it will be more consistent to return nil in case of errors. Will change.

Copy link
Contributor

@patrick-ogrady patrick-ogrady left a comment

Choose a reason for hiding this comment

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

🚀

@patrick-ogrady patrick-ogrady merged commit d931a3a into master Jan 12, 2021
@patrick-ogrady patrick-ogrady deleted the sgupta/tip-status-and-id branch January 12, 2021 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants