Skip to content
This repository has been archived by the owner on Aug 31, 2021. It is now read-only.

Increase logging in contract watcher #145

Merged
merged 1 commit into from
Oct 2, 2019
Merged

Conversation

rmulhol
Copy link
Contributor

@rmulhol rmulhol commented Sep 24, 2019

  • Focus on header mode
  • Add context to errors, trace guard clauses, warn on non-returned
    errors
  • Give errors distinct names so compiler will recognize if unchecked
  • Remove redundant type declarations/fix typos

Copy link
Collaborator

@i-norden i-norden left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for doing this. This reminds me I should go through the seed node branch and make sure the errors/logging there are to this standard.

tr.Poller.FetchContractData(tr.Parser.Abi(), contractAddr, "name", nil, name, tr.LastBlock)
pollingErr := tr.Poller.FetchContractData(tr.Parser.Abi(), contractAddr, "name", nil, name, tr.LastBlock)
if pollingErr != nil {
// can't return this error because "name" might not exist on the contract
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@elizabethengelman elizabethengelman left a comment

Choose a reason for hiding this comment

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

:shipit: 🚀

headerRepository.CreateOrUpdateHeader(mocks.MockHeader2)
headerRepository.CreateOrUpdateHeader(mocks.MockHeader3)
_, err := headerRepository.CreateOrUpdateHeader(mocks.MockHeader1)
Expect(err).ToNot(HaveOccurred())
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

addresses, err = r.RetrieveTokenHolderAddresses(*info)
Expect(err).ToNot(HaveOccurred())
var retrieveErr error
addresses, retrieveErr = r.RetrieveTokenHolderAddresses(*info)
Copy link
Contributor

Choose a reason for hiding this comment

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

super tiny, but I think we could probably change this to addresses, retrieveErr := r.RetrieveTokenHolderAddresses(*info) instead of defining retrieveErr on the line above.

addresses, err = r.RetrieveTokenHolderAddresses(contract.Contract{})
Expect(err).ToNot(HaveOccurred())
var retrieveErr error
addresses, retrieveErr = r.RetrieveTokenHolderAddresses(contract.Contract{})
Copy link
Contributor

Choose a reason for hiding this comment

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

here too, totally not merge blocking.

Copy link
Contributor

@Gslaughl Gslaughl left a comment

Choose a reason for hiding this comment

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

:shipit:

@rmulhol rmulhol force-pushed the contract-watcher-logging branch from ab81192 to 8a93cfb Compare October 2, 2019 18:00
- Focus on header mode
- Add context to errors, trace guard clauses, warn on non-returned
  errors
- Give errors distinct names so compiler will recognize if unchecked
- Remove redundant type declarations/fix typos
@rmulhol rmulhol force-pushed the contract-watcher-logging branch from 8a93cfb to 4505382 Compare October 2, 2019 20:14
@rmulhol rmulhol merged commit 0310431 into staging Oct 2, 2019
@rmulhol rmulhol deleted the contract-watcher-logging branch October 2, 2019 21:30
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