-
Notifications
You must be signed in to change notification settings - Fork 0
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
[SENP-648] feat(ecs-deploy): improve consistency of workflows parameters #83
Conversation
4dba87e
to
89c2952
Compare
89c2952
to
78d4aba
Compare
78d4aba
to
70718d6
Compare
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.
Mostly looks good to me, though I didn't test it so I might have missed something.
use_version_as_docker_image_tag: | ||
type: boolean | ||
default: true |
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.
Could you add the required
block to make the behaviour explicit here, I admit I don't remember which is the default and we add it whether we set it to false
or true
in the other fields.
.github/workflows/ecs-deploy-v2.yml
Outdated
current_version: ${{ steps.current_version.outputs.image_tag }} | ||
deployed_version: ${{ inputs.docker_image_tag }} | ||
former_version: ${{ steps.ecs_lookup.outputs.image_tag || steps.git_hash.output.previous }} | ||
new_version: ${{ inputs.docker_image_tag || steps.git_hash.output.current }} |
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.
Are the lint errors due to the linter having difficulty with understanding the output of the new workflow ?
146f69e
to
9c6e045
Compare
72f8f95
to
8d53dfb
Compare
ecs-deploy:
docker_image_tag
is now optional on theecs-deploy
deploy workflow, this will make applications that have a static image tag easier to configure.docker_image_tag
is now deprecated in favour ofversion
, this term is closer to the level of abstraction of this workflow.version
one might setuse_version_as_docker_image_tag
tofalse
to avoid the version being passed as docker_image_tag to the terraform apply (default istrue
, this is the most common usage)deployment-notification:
current_version
is now deprecated in favour offormer_version
anddeployed_version
in favour ofnew_version
, this should reduce the ambiguity when using this action.