Skip to content

Correct a typo in VpcEndpointServiceAllowedPrinciple #5396

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

t0yv0
Copy link
Member

@t0yv0 t0yv0 commented Apr 3, 2025

SDK-generated classes are now VpcEndpointServiceAllowedPrincipal, old classes still work but warn to upgrade to the correct spelling.

Fixes #5394

SDK-generated classes are now VpcEndpointServiceAllowedPrincipal, old classes still work but warn to upgrade to the
correct spelling.
Copy link

github-actions bot commented Apr 3, 2025

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • ec2/vpcEndpointServiceAllowedPrincipal.VpcEndpointServiceAllowedPrincipal

Maintainer note: consult the runbook for dealing with any breaking changes.

Copy link

codecov bot commented Apr 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 24.35%. Comparing base (6eef02b) to head (f260179).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5396   +/-   ##
=======================================
  Coverage   24.35%   24.35%           
=======================================
  Files         364      364           
  Lines      145768   145768           
=======================================
  Hits        35508    35508           
  Misses     110162   110162           
  Partials       98       98           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@@ -5652,6 +5652,16 @@ compatibility shim in favor of the new "name" field.`)
prov.Resources[k] = v
}

// Correct a typo: Principle -> Principal.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe you actually need this. I believe that if you edit the definition in resources.go above you'd get the right thing since we have AutoAliasing and the old name is saved in the bridge-metadata

Copy link
Member Author

Choose a reason for hiding this comment

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

Since a human is doing it it sounds better to have it explicit tbh.

Copy link
Contributor

@VenelinMartinov VenelinMartinov left a comment

Choose a reason for hiding this comment

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

The advantage to letting our normal mechanism handle this is that there's less stuff in resources.go

The change is in the git history, so there is still a record but anyone reading the mapping file will just see the correct thing.

Up to you

@t0yv0
Copy link
Member Author

t0yv0 commented Apr 3, 2025

The change is in the git history, so there is still a record but anyone reading the mapping file will just see the correct thing.

I have my doubts whether those are readable without tooling. Anyway not worth reworking IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VpcEndpointServiceAllowedPrinciple should be VpcEndpointServiceAllowedPrincipal
2 participants