feat(deploy/ecs): add ecs support to halyard #916
Conversation
|
Nicely done! To generate the docs that we will promote to spinnaker.io on the next stable release of halyard, can you run |
|
|
||
| @Override | ||
| public void validate(ConfigProblemSetBuilder p, EcsAccount n) { | ||
| p.addProblem(Severity.WARNING, "Very limited validation for the ECS provider has been implemented."); |
There was a problem hiding this comment.
Could you rephrase this to "This only validates that a corresponding AWS account has been created for your ECS account."
I think that'll potentially avoid some questions down the road when people see a WARNING in their deploy logs.
There was a problem hiding this comment.
@lwander Sorry, I'm new at this. I've squashed my commits and force pushed which seems to be the correct approach based on the contributing page, but that makes the comments in this PR look odd since they no longer reflect the code. Should I instead be just adding new commits, and they can be squashed when merged into master ?
Anyway, added the docs and updated the warning message.
There was a problem hiding this comment.
No worries, that's the right approach. I (personally) prefer keeping all commits separate until we merge for clarity, maybe we can update the contributor guide to reflect that.
melbit-michaelw
force-pushed
the
ecs-support
branch
from
05337f6
to
dd2e7fc
Compare
PR to add ECS support into Halyard. This also requires Deck PR 5190