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

(ec2): typo variable name "clientVpnEndoint" #13810

Closed
do-su-0805 opened this issue Mar 26, 2021 · 8 comments
Closed

(ec2): typo variable name "clientVpnEndoint" #13810

do-su-0805 opened this issue Mar 26, 2021 · 8 comments
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@do-su-0805
Copy link

do-su-0805 commented Mar 26, 2021

❓ General Issue

The Question

Environment

  • Module Version: v1.95.0

Other information

P.S. Probably here too https://github.com/aws/aws-cdk/pull/12234/files#diff-53d5a35c408bb5bcd8c3b8e135dbd00660447763e9003af414536ab603d4a25dR712. Correctly "addAuthorizationRule()"

@do-su-0805 do-su-0805 added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Mar 26, 2021
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Mar 26, 2021
@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 30, 2021

Great finds! If this is about released code, the proper solution would be to add a new member with the right name and add the @deprecated tag to the typo'ed members.

Mind submitting a PR for that?

@rix0rrr rix0rrr added bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2 and removed guidance Question that needs advice or information. labels Mar 30, 2021
@do-su-0805
Copy link
Author

do-su-0805 commented Mar 30, 2021

Thank you for your reaction and for telling me how to fix it.It was good.
I'm sorry I just found it and haven't prepared a fix yet. Please submit the correction PR.

( I'll try good first issuenext time. thx! )

@do-su-0805
Copy link
Author

Sorry, It seems that you misunderstood your intention.
Unfortunately, I have no time to deal with this issue temporarily. I'd like to ask you leave it opened or close this for now. I'll create a PR in the next month or so.

@BLasan
Copy link
Contributor

BLasan commented Apr 5, 2021

@rix0rrr @do-su-0805 If you guys don't mind shall I send a PR for this?

@do-su-0805
Copy link
Author

@BLasan Please by all means. I'm sorry I couldn't implement it.

@cokia
Copy link
Contributor

cokia commented May 22, 2021

I made a PR because this is not being solved.
I need a lot of feedback because this is my first contribution to this project
Thanks

@ryparker ryparker removed the needs-triage This issue or PR still needs to be triaged. label Jun 1, 2021
@rix0rrr rix0rrr assigned njlynch and unassigned rix0rrr Jun 3, 2021
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 11, 2021
aws#13810

Require either clientVpnEndoint (which has a typo and is deprecated in
the related PR for this issue) or clientVpnEndpoint, but not both.
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 11, 2021
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 11, 2021
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 11, 2021
aws#13810

This is to fix a linter error. The linter prefers expect and toThrow
over try-catch when statements in tests are expected to throw.
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 12, 2021
aws#13810

I am only testing the logic in the constructor that makes sure only one
of ClientVpnEndoint (deprecated, typo) and ClientVpnEndpoint are
supplied. This commit removes setup and tests that I had copied over
from another test, code that does not relate to the behavior under test.
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 12, 2021
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 12, 2021
aws#13810

This modifies the annotations for clientVpnEndoint to match with
elsewhere in aws#14902.

The reasoning for the deprecation ("by typo") can be left to the commit
and PR history.
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 13, 2021
aws#13810

We want existing usages of the deprecated prop clientVpnEndoint [sic] to
still work.
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
aws#13810

We are deprecating the prop ClientVpnEndoint [sic] due to a typo. This
commit sets up the ClientVpnRoute constructor to require one or the
other of clientVpnEndoint (the deprecated name) or clientVpnEndpoint.
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
aws#13810

Require either clientVpnEndoint (which has a typo and is deprecated in
the related PR for this issue) or clientVpnEndpoint, but not both.
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
aws#13810

This is to fix a linter error. The linter prefers expect and toThrow
over try-catch when statements in tests are expected to throw.
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
aws#13810

I am only testing the logic in the constructor that makes sure only one
of ClientVpnEndoint (deprecated, typo) and ClientVpnEndpoint are
supplied. This commit removes setup and tests that I had copied over
from another test, code that does not relate to the behavior under test.
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
aws#13810

This modifies the annotations for clientVpnEndoint to match with
elsewhere in aws#14902.

The reasoning for the deprecation ("by typo") can be left to the commit
and PR history.
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
aws#13810

We want existing usages of the deprecated prop clientVpnEndoint [sic] to
still work.
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
douglasnaphas added a commit to douglasnaphas/aws-cdk that referenced this issue Jul 18, 2021
aws#13810

We are deprecating the prop ClientVpnEndoint [sic] due to a typo. This
commit sets up the ClientVpnRoute constructor to require one or the
other of clientVpnEndoint (the deprecated name) or clientVpnEndpoint.
mergify bot pushed a commit that referenced this issue Aug 10, 2021
It is a solution to the issue #13810

Corrected the typo "clientVpnEndpoint" in the ec2 module to "clientVpnEnpoint".

The maintainer (@rix0rrr) talked about @deprecated tags on existing incorrect members in the issue thread, but I didn't work on it because I thought it was an internal reference member. (if @deprecated tag is the rule of this repository in this case, please comment).

This is my first commitment to this report. If there's anything wrong, please let me know in the comments. thanks

---- edit -----
Originally, this PR was #14835 , but I deleted my fork report by mistake, so I couldn't correct the error, so I uploaded it again.
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
It is a solution to the issue aws#13810

Corrected the typo "clientVpnEndpoint" in the ec2 module to "clientVpnEnpoint".

The maintainer (@rix0rrr) talked about @deprecated tags on existing incorrect members in the issue thread, but I didn't work on it because I thought it was an internal reference member. (if @deprecated tag is the rule of this repository in this case, please comment).

This is my first commitment to this report. If there's anything wrong, please let me know in the comments. thanks

---- edit -----
Originally, this PR was aws#14835 , but I deleted my fork report by mistake, so I couldn't correct the error, so I uploaded it again.
@jumic
Copy link
Contributor

jumic commented Sep 2, 2021

@peterwoodworth Could you please close this issue? It was resolved by PR #14902. Thanks.

@njlynch njlynch closed this as completed Sep 2, 2021
@github-actions
Copy link

github-actions bot commented Sep 2, 2021

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

david-doyle-as24 pushed a commit to david-doyle-as24/aws-cdk that referenced this issue Sep 7, 2021
It is a solution to the issue aws#13810

Corrected the typo "clientVpnEndpoint" in the ec2 module to "clientVpnEnpoint".

The maintainer (@rix0rrr) talked about @deprecated tags on existing incorrect members in the issue thread, but I didn't work on it because I thought it was an internal reference member. (if @deprecated tag is the rule of this repository in this case, please comment).

This is my first commitment to this report. If there's anything wrong, please let me know in the comments. thanks

---- edit -----
Originally, this PR was aws#14835 , but I deleted my fork report by mistake, so I couldn't correct the error, so I uploaded it again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

No branches or pull requests

7 participants