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

fix(cli): Python id parameter in init template conflicts with built-in #10874

Merged
merged 29 commits into from
Nov 9, 2020
Merged

fix(cli): Python id parameter in init template conflicts with built-in #10874

merged 29 commits into from
Nov 9, 2020

Conversation

ericzbeard
Copy link
Contributor

In Python, id() is a builtin function that can cause linter warnings and some confusion while developing stacks in Python. This change simply renames id to stack_id in the init template.

@gitpod-io
Copy link

gitpod-io bot commented Oct 14, 2020

@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Oct 14, 2020
@rix0rrr rix0rrr changed the title fix(cli) rename id to stack_id in python init template fix(cli): rename id to stack_id in python init template Oct 19, 2020
Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

This id is more generic though. I don't think stack_id is the best name to start giving people mental models. construct_id would be better, or cid if construct_id is too long to type.

@ericzbeard ericzbeard requested review from RomainMuller and removed request for RomainMuller October 20, 2020 14:17
@ericzbeard ericzbeard changed the title fix(cli): rename id to stack_id in python init template fix(cli): rename id to construct_id in python init template Oct 20, 2020
@@ -1,9 +1,9 @@
from aws_cdk import core


class %name.PascalCased%Stack(core.Stack):
class % name.PascalCased % Stack(core.Stack):
Copy link
Contributor

Choose a reason for hiding this comment

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

These autoformats are going to break things I believe.

@ericzbeard ericzbeard added the pr/work-in-progress This PR is a draft and needs further work. label Oct 26, 2020
@rix0rrr rix0rrr changed the title fix(cli): rename id to construct_id in python init template fix(cli): id parameter conflicts with built-in Oct 28, 2020
@rix0rrr rix0rrr changed the title fix(cli): id parameter conflicts with built-in fix(cli): Python id parameter conflicts with built-in Oct 28, 2020
@rix0rrr rix0rrr changed the title fix(cli): Python id parameter conflicts with built-in fix(cli): Python id parameter in init template conflicts with built-in Oct 28, 2020
@rix0rrr rix0rrr added the pr-linter/exempt-test The PR linter will not require test changes label Nov 9, 2020
@mergify
Copy link
Contributor

mergify bot commented Nov 9, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 4e9fe43
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Nov 9, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 37a149b into aws:master Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. pr/work-in-progress This PR is a draft and needs further work. pr-linter/exempt-test The PR linter will not require test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants