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

DO-1652: Add name props, remove dependabot #1367

Merged
merged 2 commits into from
May 14, 2024
Merged

Conversation

gowrizrh
Copy link
Contributor

Description of the proposed changes

  • Removed dependabot
  • Add optional name props

@gowrizrh gowrizrh self-assigned this May 13, 2024
@gowrizrh gowrizrh added the enhancement New feature or request label May 13, 2024
@@ -119,6 +119,8 @@ export class PrerenderFargate extends Construct {

const cluster = new ecs.Cluster(this, `${prerenderName}-cluster`, {
vpc: vpc,
clusterName:
props.clusterName !== undefined ? props.clusterName : undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be better to have this as props.clusterName ? props.clusterName : undefined so that it equates null as false too along with the other ternaries.
feel free to ignore and merge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that makes sense

Although all (most?) props define propName? so only assigning the actual type or undefined is possible

Keeping this for consistency's sake as most other things check explicitly for undefined but we should reconsider this in a future major version

@gowrizrh gowrizrh merged commit 85c7f11 into main May 14, 2024
1 check passed
@gowrizrh gowrizrh deleted the feature/prerender-update branch May 23, 2024 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants