Make app deploy to wait till deployment finishes
#212
Conversation
Also enabled the `CI` environment variable along with the `--dispatch` option to skip the deployment await. Improved and refined the command output.
|
First look at these box-sections - awesome!! |
|
|
||
| export const command = 'deploy'; | ||
| export const desc = 'Deploy this Saleor App repository to Vercel'; | ||
|
|
||
| export const builder: CommandBuilder = (_) => _; | ||
| export const builder: CommandBuilder = (_) => | ||
| _.option('dispatch', { |
There was a problem hiding this comment.
Maybe I'm not familiar with "dispatch" term in this context, so without reading description, I wouldn't firgure out what dispatch is.
Is this term some pattern used in other apps? Will it be understandable for community?
There was a problem hiding this comment.
We can change this. Intuitively I used a term from the queueing nomenclature i.e. putting a long-running job on an execution queue is usually called dispatching; in other words we don't wait for the result, the execution is started and run asynchronously.
There was a problem hiding this comment.
That sounds good 👍
If we have a "better name" later (maybe more known for frontend devs) we can always add alias
| ); | ||
| console.log(`\nYour Vercel URL: ${chalk.cyan(`https://${url}`)}`); | ||
| console.log(''); |
There was a problem hiding this comment.
It's easier to see newlines this way when composing messages. With the \n at the end it's a bit difficult to see. I don't have a strong preference here. It's just slightly more practical.
There was a problem hiding this comment.
How about
console.log('\n');
It's a nitpick but this way it's pretty obvious what it does. But up2u
There was a problem hiding this comment.
Nitpicks are always welcome. Here, that would be a double newline, but let's discuss it on Monday. We have some ideas for a global approach.
There was a problem hiding this comment.
Ach, ok, I'm not used to CLI too much, was using mainly Ink for that. Its fine
Also use the correct types with `Deployment`.
|
@lkostrowski PR refined. WDYT? |
|
|
||
| export const command = 'deploy'; | ||
| export const desc = 'Deploy this Saleor App repository to Vercel'; | ||
|
|
||
| export const builder: CommandBuilder = (_) => _; | ||
| export const builder: CommandBuilder = (_) => | ||
| _.option('dispatch', { |
There was a problem hiding this comment.
That sounds good 👍
If we have a "better name" later (maybe more known for frontend devs) we can always add alias
| ); | ||
| console.log(`\nYour Vercel URL: ${chalk.cyan(`https://${url}`)}`); | ||
| console.log(''); |
There was a problem hiding this comment.
How about
console.log('\n');
It's a nitpick but this way it's pretty obvious what it does. But up2u
|
|
Also enabled the
CIenvironment variable along with the--dispatchoption to skip the deployment await.Improved and refined the command output.