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

Jump to bottom

Fix capitalization of lambda.Runtime names #980

Closed
dstufft opened this issue Oct 22, 2018 · 3 comments · Fixed by #2815, #3023 or MechanicalRock/tech-radar#14 · May be fixed by MechanicalRock/cdk-constructs#5 or MechanicalRock/cdk-constructs#6
Closed

Fix capitalization of lambda.Runtime names #980

dstufft opened this issue Oct 22, 2018 · 3 comments · Fixed by #2815, #3023 or MechanicalRock/tech-radar#14 · May be fixed by MechanicalRock/cdk-constructs#5 or MechanicalRock/cdk-constructs#6
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug.

Comments

@dstufft
Copy link
Contributor

dstufft commented Oct 22, 2018

It is expected that names are going to be mangled to idiomatic names when the CDK is generated for different target languages. However, not all of the names are currently accurate in their source naming.

The one I just ran into, is @aws-cdk/aws-lambda.Runtime.NodeJS*. Because it's written as NodeJS* instead of NodeJs*, it's conceptually 3 words, Node, J, S, which gets mangled to node_j_s* when targeting Python (which uses snake casing).

We should probably fix these existing names so that they are consistent.

This is likely not going to be the last time something like this happens, so we probably want to come up with a strategy for handling these cases that doesn't break end user code, but allows us to fix these names.
@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 22, 2018

Seems useful to put a single name parser into jsii-pacmak so it can be shared by all codegens.

By the way, I'm not even sure I like node_js as a snake case. Should we just degenerate to naming hints?

@dstufft
Copy link
Contributor Author

dstufft commented Oct 26, 2018

Well CodeMaker has a name mangler built into it, and that's what I'm using here. We could add naming hints as an optional thing where if a name gets a naming hint set to it, it overrides the default mangling.

@eladb eladb added bug This issue is a bug. @aws-cdk/aws-lambda Related to AWS Lambda labels Dec 17, 2018
@eladb eladb mentioned this issue Dec 17, 2018
Closed
@eladb eladb changed the title Not all names are "well formed" Fix capitalization of Lambda's runtime static properties Mar 4, 2019
@eladb eladb changed the title Fix capitalization of Lambda's runtime static properties Fix capitalization of lambda.Runtime names Mar 4, 2019
@eladb eladb self-assigned this Apr 2, 2019
@eladb
Copy link
Contributor

eladb commented Apr 10, 2019

Blocked on the decision of #2222

@eladb eladb removed their assignment May 5, 2019
@shivlaks shivlaks self-assigned this May 31, 2019
@shivlaks shivlaks mentioned this issue Jun 11, 2019
Merged
4 tasks
shivlaks added a commit that referenced this issue Jun 12, 2019
@shivlaks
refactor(lambda): renamed the lambda.Runtime enum values from NodeJS …
10c37dd 
…to Nodejs (#2815)

Addresses the consistency of casing for runtime for other target languages.
in Python, this was previously `NODE_J_S` and now it will be `NODEJS`

**Fixes #980**

BREAKING CHANGE:
* **lambda:** `lambda.Runtime.NodeJS*` are now `lambda.Runtime.Nodejs*`
ScOut3R pushed a commit to ScOut3R/aws-cdk that referenced this issue Jun 13, 2019
@shivlaks @ScOut3R
refactor(lambda): renamed the lambda.Runtime enum values from NodeJS …
df2a6a4 
…to Nodejs (aws#2815)

Addresses the consistency of casing for runtime for other target languages.
in Python, this was previously `NODE_J_S` and now it will be `NODEJS`

**Fixes aws#980**

BREAKING CHANGE:
* **lambda:** `lambda.Runtime.NodeJS*` are now `lambda.Runtime.Nodejs*`
@shivlaks shivlaks mentioned this issue Jun 24, 2019
Merged
eladb pushed a commit that referenced this issue Jun 24, 2019
@shivlaks
refactor: rename enum-like members to ALL_CAPS (#3023)
9d19aef 
Similar to the enum naming convention, public static readonly properties
must be ALL_CAPS.

This is in accordance with the new construct library guidelines and with normal enums.

Fixes #3018
Fixes #980

BREAKING CHANGE: all public static readonly members (enum-like) have been renamed to use "ALL_CAPS" capitalization
@mhuebner mhuebner mentioned this issue Jun 26, 2019
Closed
1 task
This was referenced Aug 22, 2019
Open
Open
Open
Open
Open
This was referenced Dec 12, 2019
Closed
Closed
Closed
Merged
Closed
Closed
This was referenced Dec 12, 2019
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
This was referenced Jan 20, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor(lambda): renamed the lambda.Runtime enum values from NodeJS to Nodejs aws/aws-cdk
refactor: rename enum-like members to ALL_CAPS aws/aws-cdk
[Snyk] Upgrade @aws-cdk/aws-iam from 0.22.0 to 0.39.0 MechanicalRock/tech-radar
[Snyk] Upgrade @aws-cdk/aws-codebuild from 0.24.1 to 0.39.0 MechanicalRock/cdk-constructs
[Snyk] Upgrade @aws-cdk/aws-codepipeline from 0.24.1 to 0.39.0 MechanicalRock/cdk-constructs
[Snyk] Upgrade @aws-cdk/aws-s3 from 0.24.1 to 0.39.0 MechanicalRock/cdk-constructs
[Snyk] Upgrade @aws-cdk/aws-cloudfront from 0.15.2 to 0.39.0 MechanicalRock/cdk-products
[Snyk] Upgrade @aws-cdk/aws-codebuild from 0.15.2 to 0.39.0 MechanicalRock/cdk-products
[Snyk] Upgrade @aws-cdk/aws-codecommit from 0.15.2 to 0.39.0 MechanicalRock/cdk-products
[Snyk] Upgrade @aws-cdk/aws-codepipeline from 0.15.2 to 0.39.0 MechanicalRock/cdk-products
4 participants
@dstufft @rix0rrr @eladb @shivlaks