-
Notifications
You must be signed in to change notification settings - Fork 97
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
feat: add routing filtering to delegated routing server IPIP-484 #671
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.
CI is red, so only did a quick first-pass with some feedback inline
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #671 +/- ##
==========================================
+ Coverage 60.15% 60.36% +0.21%
==========================================
Files 241 243 +2
Lines 30712 30889 +177
==========================================
+ Hits 18475 18647 +172
- Misses 10582 10587 +5
Partials 1655 1655
|
Co-authored-by: Andrew Gillis <[email protected]>
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.
LGTM
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.
Tested in updated ipfs/someguy#82 and LGTM, small suggestions inline, but I think its ok to merge once those are addressed.
Co-authored-by: Marcin Rataj <[email protected]>
@2color we have pretty good changelog, so not the end of the world, but in future remember to squash PRs like this, to avoid noise in main branch :) (in go repos we prefer squashes) |
Sounds good! Sorry about that. Should we disable merge and rebase commits for this repo then? |
Afraid we need to keep it for the current release process: |
What's in this PR