-
Notifications
You must be signed in to change notification settings - Fork 0
feat: implement the create and deploy flows #1
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
Conversation
| ```bash | ||
| # Deploy an application | ||
| pnpm ecloud app deploy \ | ||
| npx ecloud app deploy \ |
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.
npx would need to use the actual package name, right?
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.
npx uses the named bin script from package.json, so npx ecloud works 🙏
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.
that'd only work if it's found locally, right?
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.
If the user installs the cli globally then it will be available everywhere, is that what you mean?
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.
nah, i mean people normally use npx to run packages without installing them. if it's not on their machine at all, wouldn't they need to do npx @layr-labs/ecloud-cli app deploy?
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.
Ahhh, sorry yes! It would be npx @layr-labs/ecloud-cli if its not installed, and npx ecloud if it is!
|
moving to |
| logger.info('Deploying on-chain...'); | ||
| const { appAddress: deployedAppID, txHash } = await deployApp( | ||
| // 12. Deploy the app | ||
| logger.info("Deploying on-chain..."); |
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.
this pattern of the SDK using logger.info for output (instead of exposing actionable hooks) is used throughout the codebase. imo, should expose events like onProgress({ stage: 'deploying' }) and let consumers handle presentation. logging for debug is fine, but consumers should control output format - makes it harder to build custom interfaces (progress bars, web UIs, different CLIs) when the SDK is presenting directly.
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.
Good callout! I will get this refactored!
This PR implements the
createanddeployflows with flagged inputs and interactive prompts where flags are omitted.