-
Notifications
You must be signed in to change notification settings - Fork 130
Events API: Transaction dropped, sync status, and renames #1919
Conversation
This reverts commit 95f019a.
There was a problem hiding this 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 |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
pantheon/src/test/java/tech/pegasys/pantheon/services/PantheonEventsImplTest.java
Show resolved
Hide resolved
@@ -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)); |
There was a problem hiding this comment.
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...
PR description
Adds the ability to listen to SyncStatus changes and transaction dropped events to the
PantheonEvents
APIFixed Issue(s)