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

Required physical names should be "string" and not "PhysicalName" #2971

Closed
eladb opened this issue Jun 20, 2019 · 0 comments · Fixed by #2972 or MechanicalRock/tech-radar#14 · May be fixed by MechanicalRock/cdk-constructs#5, MechanicalRock/cdk-constructs#6 or MechanicalRock/cdk-constructs#7

Comments

@eladb
Copy link
Contributor

eladb commented Jun 20, 2019

If an L2 physical name is required, it's type should actually be a string instead of PhysicalName since users would have to specify it.

eladb pushed a commit that referenced this issue Jun 20, 2019
Additional minor API cleanups in the core module (see breaking changes list).

Updated physical names linter rule to verify that physical name property name is
`cfnTypeName` and that it's type is either `PhysicalName` in case the name is
optional and `string` if the name is required (fixes #2971).

Moved all internal files in @aws-cdk/cdk under "private"

BREAKING CHANGE: The deprecated `app.run()` has been removed (use `app.synth()`).
* **core:** `CfnElement.refAsString` renamed to `ref` of `string` type. The `IResolvable` version have been removed.
* **core:** `Resource.physicalName` is now a `string` instead of `PhysicalName`. If a physical name should be generated during deployment (i.e. not supplied to L1), the string will synthesize to `undefined`.
* **core:** `IStringValue` renamed to `IStringProducer`
* **core:** `Include` renamed to `CfnInclude`
* **core:** `Cfn` prefix was added to the following types:
  `CfnCreationPolicy`,
  `CfnResourceAutoScalingCreationPolicy`,
  `CfnResourceAutoScalingCreationPolicy`,
  `CfnDeletionPolicy`,
  `CfnUpdatePolicy`,
  `CfnAutoScalingRollingUpdate`,
  `CfnAutoScalingReplacingUpdate`,
  `CfnAutoScalingScheduledAction`,
  `CfnCodeDeployLambdaAliasUpdate`,
  `CfnTag`
  `CfnRuleAssertion`,
  `CfnDynamicReferenceProps`
* **core:** `deepMerge` is no longer exported.
* **core:** `CfnOutputProps.export` was renamed to `exportName`.
* **core:** `CfnOutput` all properties are now private
* **core:** `StringListCfnOutput` has been removed
eladb pushed a commit that referenced this issue Jun 21, 2019
Additional minor API cleanups in the core module (see breaking changes list).

Updated physical names linter rule to verify that physical name property name is
`cfnTypeName` and that it's type is either `PhysicalName` in case the name is
optional and `string` if the name is required (fixes #2971).

Moved all internal files in @aws-cdk/cdk under "private"

BREAKING CHANGE: The deprecated `app.run()` has been removed (use `app.synth()`).
* **core:** `CfnElement.refAsString` renamed to `ref` of `string` type. The `IResolvable` version have been removed.
* **core:** `Resource.physicalName` is now a `string` instead of `PhysicalName`. If a physical name should be generated during deployment (i.e. not supplied to L1), the string will synthesize to `undefined`.
* **core:** `IStringValue` renamed to `IStringProducer`
* **core:** `Include` renamed to `CfnInclude`
* **core:** `Cfn` prefix was added to the following types: `CfnCreationPolicy`, `CfnResourceAutoScalingCreationPolicy`, `CfnResourceAutoScalingCreationPolicy`, `CfnDeletionPolicy`, `CfnUpdatePolicy`, `CfnAutoScalingRollingUpdate`, `CfnAutoScalingReplacingUpdate`, `CfnAutoScalingScheduledAction`, `CfnCodeDeployLambdaAliasUpdate`, `CfnTag` `CfnRuleAssertion`, `CfnDynamicReferenceProps`
* **core:** `deepMerge` is no longer exported.
* **core:** `CfnOutputProps.export` was renamed to `exportName`.
* **core:** `CfnOutput` all properties are now private
* **core:** `StringListCfnOutput` has been removed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment