Skip to content

Conversation

@aahung
Copy link
Contributor

@aahung aahung commented Nov 1, 2021

Issue #, if available:

Description of changes:

Description of how you validated changes:

Checklist:

  • Add/update tests using:
  • make pr passes
  • Update documentation
  • Verify transformed template deploys and application functions as expected

Examples?

Please reach out in the comments, if you want to add an example. Examples will be
added to sam init through https://github.com/awslabs/aws-sam-cli-app-templates/

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have some way (functional tests?) to pass a template and ensure it fails some expected manner? Instead of requiring all this mocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Added one for the invalid template I intended to fix.

@aahung aahung force-pushed the fix-usage-plan-validation branch 4 times, most recently from 30e89e1 to 79cc24c Compare November 2, 2021 17:28
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor) it might be more user-friendly to tell them the expected type "UsagePlan" must be a dictionary rather than just saying it's invalid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good one, updated

@aahung aahung force-pushed the fix-usage-plan-validation branch from 79cc24c to aee2d61 Compare November 2, 2021 23:51
@aahung aahung merged commit e9b740f into aws:develop Nov 3, 2021
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.

3 participants