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(jsii-diff): renaming a positional argument is a breaking change in Python #2937

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

fix(jsii-diff): renaming a positional argument is a breaking change in Python #2937

wants to merge 3 commits into from
+135 −0

Conversation

RomainMuller
Copy link
Contributor

Related: #2927

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@RomainMuller RomainMuller self-assigned this Aug 4, 2021
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Aug 4, 2021
@RomainMuller RomainMuller marked this pull request as draft August 4, 2021 15:32
@RomainMuller
feat: always use root signature parameter names
c665b7e 
In languages such as Python (<3.8) and Ruby, positional parameters can
be referred to using their names, making these names part of the
function signature. In order to avoid enforing parameter name
consistency on the TypeScript source, the `jsii` compiler will always
use the root declaration's parameter names when emitting the `.jsii`
assembly file.

Related #2927
@gshpychka
Copy link

@RomainMuller do you think this has any chance of getting merged? I'm assuming the lack of progress is due to the high number of deprecations required on the CDK side.

@RomainMuller
Copy link
Contributor Author

@RomainMuller do you think this has any chance of getting merged? I'm assuming the lack of progress is due to the high number of deprecations required on the CDK side.

We are also consider alternate solutions to this...

Quite frankly... it might be "easier" to wait for Python 3.7 EOL next June, as Python 3.8+ supports "positional-only" argument notation (using the / modifier), which we could then adopt and forego this problem entirely.

@gshpychka
Copy link

@RomainMuller do you think this has any chance of getting merged? I'm assuming the lack of progress is due to the high number of deprecations required on the CDK side.

We are also consider alternate solutions to this...

Quite frankly... it might be "easier" to wait for Python 3.7 EOL next June, as Python 3.8+ supports "positional-only" argument notation (using the / modifier), which we could then adopt and forego this problem entirely.

Got it. But that would still be a breaking change, some folks may be using them as keyword arguments.

@RomainMuller
Copy link
Contributor Author

Got it. But that would still be a breaking change, some folks may be using them as keyword arguments.

Yeah... Nothing is ever that easy... we might be able to strategize around this however.

@mrgrain mrgrain mentioned this pull request Nov 14, 2023
Open
@iliapolo iliapolo mentioned this pull request Sep 9, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RomainMuller @gshpychka