Skip to content
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

feat(servicediscovery): AWS Cloud Map construct library #1804

Merged
merged 22 commits into from
Mar 13, 2019

Conversation

SoManyHs
Copy link
Contributor

Partially addresses #1554

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

@SoManyHs SoManyHs requested a review from a team as a code owner February 20, 2019 07:15
@SoManyHs
Copy link
Contributor Author

SoManyHs commented Feb 20, 2019

Need to add a README and probably a few more tests, but wanted to get this up for initial review.

This is preliminary work for ECS service discovery integration (can follow progress on WIP branch here)

@eladb eladb changed the title feat(servicediscovery): add construct lib feat(servicediscovery): AWS Cloud Map construct library Feb 20, 2019
eladb
eladb previously requested changes Feb 20, 2019
packages/@aws-cdk/aws-servicediscovery/lib/instance.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicediscovery/lib/instance.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicediscovery/lib/instance.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicediscovery/lib/instance.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicediscovery/lib/instance.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicediscovery/lib/namespace.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicediscovery/lib/service.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicediscovery/lib/service.ts Outdated Show resolved Hide resolved
@rix0rrr rix0rrr self-assigned this Feb 20, 2019
@SoManyHs SoManyHs force-pushed the servicediscovery branch 6 times, most recently from 51c4693 to ca15e65 Compare March 8, 2019 01:15
@SoManyHs SoManyHs requested review from eladb and rix0rrr March 8, 2019 08:54
packages/@aws-cdk/aws-servicediscovery/lib/namespace.ts Outdated Show resolved Hide resolved
*
* @default none
*/
healthCheckCustomConfig?: HealthCheckCustomConfig;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use the suffix config both in property name and type name

*
* @default none
*/
healthCheckConfig?: HealthCheckConfig;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove "config" suffix

packages/@aws-cdk/aws-servicediscovery/lib/service.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicediscovery/lib/service.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicediscovery/lib/service.ts Outdated Show resolved Hide resolved
@rix0rrr rix0rrr dismissed eladb’s stale review March 12, 2019 16:02

We reworked it

@jogold
Copy link
Contributor

jogold commented Mar 12, 2019

Hey @SoManyHs, @rix0rrr,

Have you seen this #1804 (comment)? Do you want to do something at this stage?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants