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

Fix warnings in AST Loader for Resource and Operation Shapes with mixins #1626

Merged
merged 6 commits into from
Feb 22, 2023

Conversation

hpmellema
Copy link
Contributor

Description of changes:
Updates the AST loader to allow both Operation and Resource shapes to have a mixin property without throwing a warning.

Previously the following warning was being emitted on building the source projection:

      |
50 |         "example.weather.operations#OperationWithMixin": {
      |                                                          ^
     = Expected an object with possible properties of `errors`, `input`, `output`, `traits`, `type`, but found additional properties: `mixins`
SUCCESS: Validated 299 shapes (WARNING: 1)

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

@hpmellema hpmellema requested a review from a team as a code owner February 21, 2023 15:33
@hpmellema hpmellema changed the title Fix mixin warnings Fix mixin warnings in AST Loader for Resource and Operation Shapes Feb 21, 2023
@hpmellema hpmellema changed the title Fix mixin warnings in AST Loader for Resource and Operation Shapes Fix warnings in AST Loader for Resource and Operation Shapes with mixins Feb 21, 2023
@syall
Copy link
Contributor

syall commented Feb 21, 2023

Looking at the spec for mixins, services and other shapes might also have this warning, will need to make sure each of the shape types have the addMixins() call.

@hpmellema hpmellema merged commit 01a2d5b into smithy-lang:main Feb 22, 2023
@hpmellema hpmellema deleted the fix-mixin-warning branch February 22, 2023 22:20
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.

2 participants