-
Notifications
You must be signed in to change notification settings - Fork 796
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
better handling of deployments #666
Conversation
Thanks for this @technophile-04 ! I'm all in with 1). But not sure if 2) is worth it: the assume-unchanged is a bit hacky and another extra file in I'll think about it a bit more tho. |
Yup yup completely agree !!! Also, I feel 2. is not worth it due to the extra addition of files which kind of bloats Also doing 1.) I think it solves the main issue nicely and we don't need 2.) at all for that. Planning to update this PR to just gitingore |
This reverts commit 18deb1e.
Reverted all the changes, Just added |
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 great! Thanks @technophile-04
@technophile-04 this is great!! Thanks!! |
Description:
Summary from #369:
Current Issue:
The
deployments
folder in Hardhat is.gitignored
, leading to deployment data being stored locally. This causes issues when different team members deploy contracts on different networks (like Polygon and Arbitrum). ThedeployedContracts
file in Next.js only reflects the latest deployment, losing data about previous deployments on other networks. Even while one person is working, I think its good to keep track ofdeployments/{non-localhost}
in git.Expected Behavior:
Ideally, deploying a contract on a new network should append data to the
deployedContracts
file without losing existing data from other networks.Solution discussed:
deployments
folder in Hardhat (deployments/localhost
will still be ignored).deployedContracts
file into two files:deployedLocalhostContracts
(git ignored) anddeployedContracts
. The former will be tracked by git, and the latter will be ignored by git.Problem with 2.:
To make types work properly
deployedLocahostContracts
needs to be present so that typescript can infer the types. But ifdeployedLocalhostContracts
is git ignored, it won't be present on a fresh clone. Also we can't requiredeployedLocalhostContracts
optionally since again TS won't be able to infer the types. checkout this comment by Samuel. Also, another thing is we need to pushdeployedLocalhostContracts
(with empty object) to github so that CI checks pass other wise it will fail giving an error (no such file is present).This PR summary:
Added
git update-index --assume-unchanged packages/nextjs/contracts/deployedLocalhostContracts.ts
inpostinstall
script. This is to prevent the file from being tracked by git, so any changes made to this file will be ignored. This ensures we don't get annoyingdeployedLocalhostContracts
modified messages when we rungit status
and also prevents us from pushing the modified file to github.Also added a step in pre-commit hook that reverts any changes done to
deployedLocalhostContracts.ts
(if any). We can probably remove this.Lol not sure if it looks too hacky, this was just a try and we could completely close it 🙌 / iterate on other approaches to handle this
Testing:
To test it you will need to create a new clone and test since
postinstall
script only runs when dependency tree is changed.Try this :
And then
cd test-deployments
,yarn install
and other SE-2 commands.