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.

Sign up for GitHub

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

chore(servicecatalogappregistry): providing default values for CreateApplicationTargetOptions #22890

Merged
merged 4 commits into from
Nov 15, 2022
Merged

chore(servicecatalogappregistry): providing default values for CreateApplicationTargetOptions #22890

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3125d34
Providing default values for CreateApplicationTargetOptions
Nov 11, 2022
4d9614b
Updated Stack and Application description
Nov 14, 2022
2a73ab2
Updated Default description for applicationDescription
Nov 15, 2022
1bb1996
Merge branch 'main' into defaultApplication
rix0rrr Nov 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-servicecatalogappregistry/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ const app = new App();
const associatedApp = new appreg.ApplicationAssociator(app, 'AssociatedApplication', {
applications: [appreg.TargetApplication.createApplicationStack({
applicationName: 'MyAssociatedApplication',
// 'AppRegistry Application to which all stacks inside this cdkApp will get associated.' is the default
applicationDescription: 'Associated Application description',
stackName: 'MyAssociatedApplicationStack',
// AWS Account and Region that are implied by the current CLI configuration is the default
env: { account: '123456789012', region: 'us-east-1' },
})],
});
Expand Down
8 changes: 6 additions & 2 deletions packages/@aws-cdk/aws-servicecatalogappregistry/lib/target-application.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export interface CreateTargetApplicationOptions extends TargetApplicationCommonO
/**
* Application description.
*
* @default - No description.
* @default - AppRegistry Application to which all stacks inside this cdkApp will get associated.
*/
readonly applicationDescription?: string;
}
Expand Down Expand Up @@ -90,10 +90,14 @@ class CreateTargetApplication extends TargetApplication {
}
public bind(scope: Construct): BindTargetApplicationResult {
const stackId = this.applicationOptions.stackId ?? 'ApplicationAssociatorStack';
(this.applicationOptions.description as string) =
this.applicationOptions.description || 'Stack containing AppRegistry Application';
(this.applicationOptions.env as cdk.Environment) =
this.applicationOptions.env || { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION };
const applicationStack = new cdk.Stack(scope, stackId, this.applicationOptions);
const appRegApplication = new Application(applicationStack, 'DefaultCdkApplication', {
applicationName: this.applicationOptions.applicationName,
description: this.applicationOptions.applicationDescription,
description: this.applicationOptions.applicationDescription || 'AppRegistry Application to which all stacks inside this cdkApp will get associated.',
Copy link
Contributor

Choose a reason for hiding this comment

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

When you look at the description in the console, the words "this cdkApp" are meaningless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are trying to tell the purpose of the creation of the app. I agree this cdkApp is useless. How about this ?
AppRegistry Application to which all stacks inside the cdkApp will get associated.

});

return {
Expand Down