-
Notifications
You must be signed in to change notification settings - Fork 2
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: restructure with lockup dir #12
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.
Renaming this repo to deployment
can be synchronised with renaming v2-core
to lockup
. So, feel free to merge this and open another issue about renaming repo.
we need to also update the hyperlinks in docs |
Right. So should we mark it as draft then to prevent accidental merging of this PR |
we could, but some clarifications, doesn't "draft" mean that the work for the individual PR is not finished? |
As per https://github.blog/news-insights/product-news/introducing-draft-pull-requests/, Draft PR can be used to send various signals one of which is to block merging the pull request. |
refactor: remove "V2" refactor: divide by mainnets and testnets
ff95fff
to
ad1d2c0
Compare
* feat: add flow deployments * update commit
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.
Here is another review after merging #14.
- ZKsync broadcasts and ZKsync artifacts are missing. ZKsync artifacts should be different because of hardhat requirement (similar to lockup directory).
- We should update the Scripts section in the README file as the bash scripts are no longer used to make deployments.
- Link to Flow npm package was missing from README which I have added in 61e0b2f.
unfortunetely i didn't have them this time, i don't know why
will add them |
Co-authored-by: smol-ninja <shubhamy2015@gmail.com>
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.
Should we merge it @andreivladbrg given we are going live in a few hours? We will then have to update the links in docs as well. |
Closes #10, #11, #13
Important
The merge of this PR should be synchronized with renaming this repo to
deployments
(removing "V2") and updating the documentation in places such as this