-
Notifications
You must be signed in to change notification settings - Fork 308
Wormhole stub #789
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
Wormhole stub #789
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
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 @jayantk is more familiar with your progress on this and is more suitable to review this PR.
I like the pipeline concept but the implementation seems very complex and I cannot understand the rationale behind it. I think we can make it simpler while preserving the good behaviours that it has.
We can use the same thing for ETH too.
p.s: fun fact: pipeline here (with a single step) reminds me of Migrations in truffle :D The difference is that they store the last executed step on-chain instead of a local file :))
jayantk
left a 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.
I read through this and I think there are many ways you can simplify this. It's probably better discussed live, but here's a quick summary:
- Instead of parallelizing across steps (which are chains) within the Stages of the Pipeline, it would be simpler to instantiate a separate Pipeline for each chain, then run those pipelines asynchronously in parallel. If you make this change, it will simplify a lot of things, and it will let you move a lot of application-specific logic out of the generic pipeline code.
- pipeline.run() should be idempotent, so if it fails, you can rerun it and it automatically picks up where it left off. You're already storing the state it produces in the filesystem, so you should build the processing logic around that (read the filesystem, decide what to do, run a step, write out, then loop). You can name the steps something like 1_deploy_wormhole to make it easy to figure out what's next. (also, if you do this, you should commit the migration results to the repo, so that anyone can pick up the code and resume migrating from where you left off)
This would actually be a good thing to pair on if we can find some time later today or tomorrow.
ali-behjati
left a 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.
Amazing ser. 🚢 it
jayantk
left a 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.
awesome this is much clearer this way. I left you some minor comments but feel free to merge whenever you're ready.
This PR contains