You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@technophile-04 Your proposed solution alleviates the pain points I was having and I think its a more than satisfactory solution. However, it is not a perfect solution to me, but it may just end up being a nitpick.
yarn deploy: To deploy YourContract.sol yarn deployFactory: To deploy YourContractUpgradeable.sol and YourContractFactory.sol.
But the problem with this approach is that it ovverrides abi's generated in deployedContracts.ts and keep only YourContractUpgradeable and YourContractFactory (since it was run last).
*original quote modified here to fit example below.
With the above example, I personally see each deploy script as it's own configured deployment. Thus, overriding the generated abi's should be expected. For instance, If I am only interacting with YourContract then I should call yarn deploy, but if I am interacting with the factory and upgradeable then I should call deployFactory.
The important thing here is that this approach allows for several "deployment configurations" to co-exist and the developer can pick and choose which one most necessarily fits their current situation. EDIT: The idea of changing package.json instead of Deploy.s.sol fits better with me as well, as it feels less intrusive. /EDIT As opposed to @technophile-04's solution, where they need to modify the deploy.s.sol script to fit their needs, potentially erasing important details and coming up with hacks (commenting out code) to reach the desired functionality. Again, I think it is a great solution, but we can do slightly better!
Even if the "recommended" method ends up being a single Deploy script, then I think we should still go through with the changes proposed in the PR. As, it inherently allows the system to be more flexible, and could cover an edge case or two where the developer came to the same conclusion as I and had hit a wall, where otherwise, they would've been able to continue onward even with the "improper" method.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
Original rationale and discussion found HERE.
@technophile-04 Your proposed solution alleviates the pain points I was having and I think its a more than satisfactory solution. However, it is not a perfect solution to me, but it may just end up being a nitpick.
*original quote modified here to fit example below.
With the above example, I personally see each deploy script as it's own configured deployment. Thus, overriding the generated abi's should be expected. For instance, If I am only interacting with
YourContract
then I should callyarn deploy
, but if I am interacting with the factory and upgradeable then I should calldeployFactory
.The important thing here is that this approach allows for several "deployment configurations" to co-exist and the developer can pick and choose which one most necessarily fits their current situation. EDIT: The idea of changing package.json instead of Deploy.s.sol fits better with me as well, as it feels less intrusive. /EDIT As opposed to @technophile-04's solution, where they need to modify the deploy.s.sol script to fit their needs, potentially erasing important details and coming up with hacks (commenting out code) to reach the desired functionality. Again, I think it is a great solution, but we can do slightly better!
Even if the "recommended" method ends up being a single Deploy script, then I think we should still go through with the changes proposed in the PR. As, it inherently allows the system to be more flexible, and could cover an edge case or two where the developer came to the same conclusion as I and had hit a wall, where otherwise, they would've been able to continue onward even with the "improper" method.
Beta Was this translation helpful? Give feedback.
All reactions