-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extend getpeers RPC method to provide useragent and last known height #3481
Conversation
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.
The PR was updated, so it's only a part of my review.
@AnnaShaleva @roman-khimov Removed |
Re-pushed most recent commit with sign-off because I forgot :) |
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.
Please, add neorpc:
prefix to the commit message. Ref. https://github.com/nspcc-dev/.github/blob/master/git.md#commit-messages.
@AnnaShaleva Thanks Anna, changes made. |
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.
Otherwise LGTM!
54c91d9
to
84808b8
Compare
@AnnaShaleva Think I got there eventually. Sorry for the trouble, I do very much appreciate the chance to practice a little :) |
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.
Otherwise LGTM, @roman-khimov, could you take a look?
DCO check is failing, but it's just too strict. I think we may skip it for this commit since it has a valid sign-off note:
|
@EdgeDLT, could you please fix the latest comments? There's not much left, and then we can merge. |
Signed-off-by: edgedlt <[email protected]>
@AnnaShaleva Pushed the changes. |
@EdgeDLT, could you please fix the tests? It can be checked with
|
@AnnaShaleva Sorry for the oversight, fixed! |
Signed-off-by: edgedlt <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3481 +/- ##
==========================================
- Coverage 86.19% 86.12% -0.07%
==========================================
Files 331 331
Lines 38482 38541 +59
==========================================
+ Hits 33168 33193 +25
- Misses 3782 3819 +37
+ Partials 1532 1529 -3 ☔ View full report in Codecov by Sentry. |
Close #3480
I have
OldPeerHeight
because I followedpeers.go
as an example, but I expect it may be pointless here. Please remove if so.