-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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
Remove code in preparation for v5.0 #4258
Conversation
🦋 Changeset detectedLatest commit: 41732d3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
I wouldn't have a migration script this time because I expect that it will be very few renamed/moved files, we should list it in CHANGELOG.md under "How to upgrade from 4.x". I would consider merging all the removal changesets into one, what do you think? |
contracts/crosschain/README.adoc
Outdated
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.
We need to explain the rationale for some of these removals, I think particularly this one, perhaps both in the PR description and the changesets/changelog.
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.
So what's the rationale? There's none in the PR description and the changelog only states the removal 😆 Are these bridges or the removed contracts unsafe? Did universal bridges become so good that there's no reason to support custom ones?
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 rationale is basically that the contracts had really low usage and the Crosschain landscape went in a different direction, this includes other bridges' technology and use cases.
I think the changeset is fine as it is right now, perhaps some of these notes may be added to the changelog in a different fashion other than a changeset entry.
Removing History has been on my mind, but it was not listed anywhere, and we did not discuss it, so I wanted to bring it up latter. But if you agree, lets do it! @ernestognw, is that ok with you as well ?
The PR is a bit old, an might create some conflicts. Possibly easier to re-do it here independently. |
I made this new branch but did not open a new PR https://github.com/RenanSouza2/openzeppelin-contracts/tree/next-v5.0-Checkpoint-history-v2 |
I closed that one myself for this reason and I also didn't want to spam PRs |
What syntax would you use? I think we should have one line per "important" removal. |
Fixes #3918
Fixes LIB-824
Fixes LIB-825
Fixes LIB-826
This PR removes:
draft-xxx
filesNote:
We keep the IERC777 interfaces in the
contracts/interfaces
folder. Should plan a migration script that rewrite the import part for these ?PR Checklist
npx changeset add
)