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

Events API: Transaction dropped, sync status, and renames #1919

Merged
merged 24 commits into from
Sep 11, 2019
Merged

Events API: Transaction dropped, sync status, and renames #1919

merged 24 commits into from
Sep 11, 2019

Conversation

RatanRSur
Copy link
Contributor

PR description

Adds the ability to listen to SyncStatus changes and transaction dropped events to the PantheonEvents API

Fixed Issue(s)

@RatanRSur RatanRSur changed the title Events API: Transaction dropped, sync status, and other renames Events API: Transaction dropped, sync status, and renames Sep 11, 2019
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.

Looks good to me.

@@ -49,17 +51,22 @@ public SyncState(final Blockchain blockchain, final EthPeers ethPeers) {
});
}

private void publishSyncStatus() {
@VisibleForTesting
Copy link
Contributor

Choose a reason for hiding this comment

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

(discussion) : why @VisibleForTesting if public ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know this before but @VisibleForTesting is for documentation purposes only! I used to think it actually changed the visibility for the tests in some special way. It's to communicate that if not for the tests, this method would be less visible.

@RatanRSur RatanRSur merged commit b454627 into PegaSysEng:master Sep 11, 2019
@RatanRSur RatanRSur deleted the event-stream-api branch September 11, 2019 12:42
@@ -141,6 +141,7 @@ public void shouldNotBeReadyWhenCustomMaxBlocksBehindIsInvalid() {
}

private Optional<SyncStatus> createSyncStatus(final int currentBlock, final int highestBlock) {
return Optional.of(new SyncStatus(0, currentBlock, highestBlock));
return Optional.of(
new tech.pegasys.pantheon.ethereum.core.SyncStatus(0, currentBlock, highestBlock));
Copy link
Contributor

Choose a reason for hiding this comment

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

If only java had type aliases on import...

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.

3 participants