-
Notifications
You must be signed in to change notification settings - Fork 446
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
refactor!: move autonat into separate package #2107
Conversation
Implementation notes:
|
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 think this could have been two independent PRs, one for error-code refactor and one for moving autoNAT into it's own package. It should work still, thanks!
Let's hold off on merging any breaking changes until #2074 is merged |
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 think we should move the error codes and messages into @libp2p/interface/errors first in a separate PR, given that touches more codepaths, if that is successfully merged then we know we can remove other packages more easily.
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.
Comments inline. I've targeted the v1 release branch - please can you add some copy to the migration guide to cover these breaking changes.
chore: fix type errors chore: move more autonat references chore: fix documentation chore: fix linter errors chore: fix deps chore: replace error codes and messages chore: fix linter errors chore: fix error import statements chore: fix build error
f464828
to
36de279
Compare
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, left one comment.
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.
almost there, just some minor corrections
Can you please take over this PR and fix up the migration guide the way you want? |
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Co-authored-by: chad <chad.nehemiah94@gmail.com>
Description
Notes & open questions
Change checklist