Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Remove EthTaskChainDownloader and supporting code #1373

Merged
merged 9 commits into from
May 1, 2019

Conversation

ajsutton
Copy link
Contributor

PR description

Remove the old EthTaskChainDownloader class and code that is only used by it (plus a few other bits of unused code detected by the static analysis).

We should merge #1372 and #1344 first.

@ajsutton ajsutton marked this pull request as ready for review April 30, 2019 21:56
Copy link
Contributor

@AbdelStark AbdelStark left a comment

Choose a reason for hiding this comment

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

Few optional comments. Mostly on unit tests.
LGTM

}

@Override
public int hashCode() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional. Maybe we could test equals and hashCode. Although it is non blocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Equals is only used by tests and hash code isn't used (but should exist to match equals just for good measure). I really only updated them to match our code style which is the generated code.

return String.format(
"maxGetBlockHeaders=%s\tmaxGetBlockBodies=%s\tmaxGetReceipts=%s\tmaxGetReceipts=%s",
maxGetBlockHeaders, maxGetBlockBodies, maxGetReceipts, maxGetNodeData);
return MoreObjects.toStringHelper(this)
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional. Unit test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

toString is always just for debugging purposes so the only time we should test it is when there's sensitive info that definitely shouldn't appear in a log (ie anything containing a password).

@ajsutton ajsutton merged commit e5d34e5 into PegaSysEng:master May 1, 2019
@ajsutton ajsutton deleted the remove-eth-task-downloader branch May 1, 2019 20:11
notlesh pushed a commit to notlesh/pantheon that referenced this pull request May 4, 2019
notlesh pushed a commit to notlesh/pantheon that referenced this pull request May 14, 2019
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.

2 participants