Skip to content

feat(region-info): throw ValidationError instead of untyped Errors #33384

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

Merged

Conversation

mrgrain
Copy link
Contributor

@mrgrain mrgrain commented Feb 11, 2025

Issue

Relates to #32569

Description of changes

ValidationErrors where apprporiate

Note that files in the build-tools directory are excluded from the linter rules, since they are only used at build time.
I moved the before function to build-tools because it's not exported and only used at build time. Importing the core module to a build-time script doesn't work as it would add A LOT of additional files to the build-time hot path and that just completely blows the build script up 💥 (aside from it being totally unnecessary anyway).

Describe any new or updated permissions being added

n/a

Description of how you validated changes

Existing tests. Exemptions granted as this is a refactor of existing code.

Checklist


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

@aws-cdk-automation aws-cdk-automation requested a review from a team February 11, 2025 11:10
@github-actions github-actions bot added the p2 label Feb 11, 2025
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Feb 11, 2025
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

(This review is outdated)

@mrgrain mrgrain added pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-test The PR linter will not require test changes pr-linter/exempt-integ-test The PR linter will not require integ test changes labels Feb 11, 2025
Copy link
Contributor Author

Choose a reason for hiding this comment

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

UnscopedValidationError because this module doesn't operate on scopes

@aws-cdk-automation aws-cdk-automation dismissed their stale review February 11, 2025 11:25

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

iankhou
iankhou previously approved these changes Feb 11, 2025
@mrgrain mrgrain marked this pull request as draft February 12, 2025 10:47
@mrgrain mrgrain force-pushed the feature/featregion-info-throw-validationerror-instead-of-u branch from 038607b to 12b1a74 Compare May 29, 2025 13:59
@mrgrain mrgrain force-pushed the feature/featregion-info-throw-validationerror-instead-of-u branch from 12b1a74 to dfe9726 Compare May 29, 2025 14:26
@mrgrain mrgrain dismissed iankhou’s stale review May 29, 2025 14:59

super old approval

@mrgrain mrgrain marked this pull request as ready for review May 29, 2025 15:01
@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 29, 2025
Copy link
Contributor

mergify bot commented May 30, 2025

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

@aws-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 30, 2025
Copy link
Contributor

mergify bot commented May 30, 2025

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

Copy link
Contributor

mergify bot commented May 30, 2025

Thank you for contributing! Your pull request will be updated from main 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 303daf2 into main May 30, 2025
16 checks passed
@mergify mergify bot deleted the feature/featregion-info-throw-validationerror-instead-of-u branch May 30, 2025 12:26
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: f4a2217
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

Copy link
Contributor

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
contribution/core This is a PR that came from AWS. p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exempt-readme The PR linter will not require README changes 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.

5 participants