Skip to content

Conversation

@cnlangzi
Copy link
Contributor

@cnlangzi cnlangzi commented Jun 8, 2022

Changes

Fixes

Tests

Tasks to complete before merging PR:

  • Ensure system tests are passing. If not Run them manually to check for any regressions 📋
  • Do any new system tests need added to test this change? do any existing system tests need updated? If so create a PR at 0chain/system_test
  • Merge your system tests PR to master AFTER merging this PR

Associated PRs (Link as appropriate):

@codecov-commenter
Copy link

Codecov Report

Merging #718 (fc3a43d) into staging (7b9053d) will increase coverage by 0.11%.
The diff coverage is 0.00%.

@@ Coverage Diff @@ ## staging #718 +/- ## =========================================== + Coverage 24.06% 24.18% +0.11%  =========================================== Files 69 69 Lines 7957 7907 -50 =========================================== - Hits 1915 1912 -3  + Misses 5757 5710 -47  Partials 285 285 
Flag Coverage Δ
Unit-Tests 24.18% <0.00%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
code/go/0chain.net/blobbercore/handler/health.go 0.00% <0.00%> (ø)
code/go/0chain.net/blobbercore/handler/protocol.go 0.00% <0.00%> (ø)
...et/blobbercore/handler/object_operation_handler.go 32.44% <0.00%> (-0.29%) ⬇️
...ode/go/0chain.net/blobbercore/allocation/entity.go 0.00% <0.00%> (ø)
...e/go/0chain.net/blobbercore/allocation/protocol.go 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b9053d...fc3a43d. Read the comment docs.

@cnlangzi cnlangzi requested a review from peterlimg June 8, 2022 02:56
logging.Logger.Warn("failed to refresh blobber BaseURL on chain", zap.Error(err))
}
}
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

it's better to avoid if-then-else and replace themwith if-not-then
it's easy to read something like

if err != nil { //some code return } //other code

then

if err == nil { //some code } else { //other code }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@cnlangzi cnlangzi requested a review from dabasov June 8, 2022 11:02
Copy link
Contributor

@dabasov dabasov left a comment

Choose a reason for hiding this comment

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

lgtm

@cnlangzi cnlangzi merged commit 255f32c into staging Jun 8, 2022
@cnlangzi cnlangzi deleted the fix/heathcheck branch June 8, 2022 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants