-
Notifications
You must be signed in to change notification settings - Fork 902
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
fix(ecs): move service discovery configuration to react #8188
Conversation
The following commits need their title changed: Please format your commit title into the form:
This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here. |
e75772b
to
87b3d59
Compare
@allisaurus and @clareliguori please review |
app/scripts/modules/ecs/src/serverGroup/configure/serverGroupConfiguration.service.ts
Outdated
Show resolved
Hide resolved
app/scripts/modules/ecs/src/serverGroup/configure/wizard/serviceDiscovery/serviceDiscovery.html
Show resolved
Hide resolved
app/scripts/modules/ecs/src/serverGroup/configure/wizard/serviceDiscovery/ServiceDiscovery.tsx
Outdated
Show resolved
Hide resolved
const registryMap = this.getNameToRegistryMap(); | ||
let newServiceDiscoveryRegistry = registryMap.get(newRegistry.value); | ||
|
||
if (!newServiceDiscoveryRegistry) { |
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.
since this is firing when the registry drop-down is changed, when would we not find the registry? could newRegistry
be search input (vs. an existing option)?
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.
I was trying to maintain consistency with how the other properties are loaded in the configuration wizard currently (artifacts, container mappings, etc)
4140960
to
511b9cc
Compare
* fix(ecs): move service discovery configuration to react * Address feedback
Updated the service discovery section of the configure server group wizard to react. Created and updated a server group using both the existing UI and the updated UI and the generated json is the same.
Resolves: spinnaker/spinnaker#5482