Conversation
…r into 6425-remove-deploy-sh-script
ca61688
added
the
needs-reviewer
| | File | Purpose | Usecase | | ||
| |-----------------|--------------------------------|---------| | ||
| deployNew.sh | Deploys a new instance | Use When deploying a new instance | ||
| deployExisting.sh | Redeploys an existing instance | Use when you wish to re-run the CDK against an exising deployment, in case you're concerned there's drift. Or when you wish to upgrade and existing instance to a newer version. |
There was a problem hiding this comment.
Is the latter part of this use case necessary:
From "In case you're concerned..."
rtjd6554
left a comment
rtjd6554
left a comment
There was a problem hiding this comment.
Just thought about if use case message is too much
patchwork01
left a comment
patchwork01
left a comment
There was a problem hiding this comment.
The parts about removing the deploy.sh script look great, it looks like the only part left for that is to remove its section of the documentation, which currently has just had deploy.sh replaced with deployNew.sh.
For the other documentation changes, part of the idea was that we could make a more direct explanation of what the scripts do, with a description of the CDK commands they'll run and what else they do. There was a suggestion that could be in the table. I'm not sure if the purpose/use case setup is quite right. Do you want to adjust it or would you rather get it merged?
|
|
||
| ```bash | ||
| ./scripts/deploy/deploy.sh <instance-id> <vpc-id> <subnet-ids> ./my-instance/instance.properties | ||
| ./scripts/deploy/deployNew.sh <instance-id> <vpc-id> <subnet-ids> ./my-instance/instance.properties |
There was a problem hiding this comment.
This section isn't correct any more. It's specific to the deploy.sh script, and deployNew.sh is covered in other sections just above.
The description of this code example states this "will either create or update an instance, applying your configuration declaratively". The deployNew.sh script can only create and not update an instance.
| | File | Purpose | Usecase | | ||
| |-----------------|--------------------------------|---------| | ||
| deployNew.sh | Deploys a new instance | Use When deploying a new instance | ||
| deployExisting.sh | Redeploys an existing instance | Use when you wish to re-run the CDK against an exising deployment, in case you're concerned there's drift. Or when you wish to upgrade and existing instance to a newer version. |
There was a problem hiding this comment.
There's a typo at "Use When ...", the second word shouldn't be capitalised.
| | File | Purpose | Usecase | | ||
| |-----------------|--------------------------------|---------| | ||
| deployNew.sh | Deploys a new instance | Use When deploying a new instance | ||
| deployExisting.sh | Redeploys an existing instance | Use when you wish to re-run the CDK against an exising deployment, in case you're concerned there's drift. Or when you wish to upgrade and existing instance to a newer version. |
There was a problem hiding this comment.
I think it reads a bit oddly to start a sentence with "or" here.
| | File | Purpose | Usecase | | ||
| |-----------------|--------------------------------|---------| | ||
| deployNew.sh | Deploys a new instance | Use When deploying a new instance | ||
| deployExisting.sh | Redeploys an existing instance | Use when you wish to re-run the CDK against an exising deployment, in case you're concerned there's drift. Or when you wish to upgrade and existing instance to a newer version. |
There was a problem hiding this comment.
There are typos at "scrips" and "uscases".
|
|
||
| The `deployExisting.sh` script can be used to bring an existing instance up to date. This will upload any jars | ||
| that have changed, update all the docker images, and perform a `cdk deploy`. | ||
| The `deployExisting.sh` script can be used to re-run the CDK against an existing instance of sleeper or update it to a newer version. |
There was a problem hiding this comment.
Sleeper should be capitalised here.
|
|
||
| The `deployExisting.sh` script can be used to bring an existing instance up to date. This will upload any jars | ||
| that have changed, update all the docker images, and perform a `cdk deploy`. | ||
| The `deployExisting.sh` script can be used to re-run the CDK against an existing instance of sleeper or update it to a newer version. |
There was a problem hiding this comment.
This implies that updating it to a newer version is something different from re-running the CDK, but it's a result of re-running the CDK.
patchwork01
removed
the
needs-reviewer
3 participants
Make sure you have checked all steps below.
Issue
Feature". Note that before an issue is finished, you can still make a pull request by raising a separate issue
for your progress.
Tests
Documentation
separate issue for that below.