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

chore(smithy): Codegen improvements #3439

Merged
merged 6 commits into from
Jul 20, 2023
Merged

Conversation

dnys1
Copy link
Contributor

@dnys1 dnys1 commented Jul 19, 2023

  • Adds further override capabilities for shapes
  • Aligns @default and @required behavior more closely to the spec
  • Only generates _init if necessary
  • Removes custom test generation for V1 IDL

Dillon Nys added 6 commits July 19, 2023 12:37
- Adds further override capabilities for shapes
- Aligns `@default` and `@required` behavior more closely to the spec
- Only generates `_init` if necessary
Since this tests very specific V2 features, the V1 transformation doesn't provide any value and prevents certain usages.
@dnys1 dnys1 marked this pull request as ready for review July 20, 2023 01:52
@dnys1 dnys1 requested a review from a team as a code owner July 20, 2023 01:52
@dnys1 dnys1 added the pr:squash PR should be submitted with a sqaush commit label Jul 20, 2023
@@ -1033,20 +1033,6 @@ updates:
- dependency-name: "amplify_lints"
- dependency-name: "smithy_aws"
- dependency-name: "aws_signature_v4"
- package-ecosystem: "pub"
directory: "packages/smithy/goldens/lib/custom"
Copy link
Member

Choose a reason for hiding this comment

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

Q: why removing the packages/smithy/goldens/lib/custom and not others in packages/smithy/goldens/lib e.g. packages/smithy/goldens/lib/awsQuery or packages/smithy/goldens/lib/awsJson1_1 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

custom has smithy tests I wrote by hand. The v1 / v2 split is meant to track differences in Smithy IDL versions since the AWS-vended Smithy models were (in the past) not fully migrated to V2. I think they've all been migrated at this point, so we can probably remove all v1 soon.

However, for the handwritten tests, it's not necessary to test V1 since many of the tests are specifically testing V2 features.

@dnys1 dnys1 merged commit e33b25c into main Jul 20, 2023
463 of 468 checks passed
@dnys1 dnys1 deleted the fix/smithy/codegen-improvements branch July 20, 2023 19:47
dnys1 added a commit that referenced this pull request Jul 27, 2023
* chore(smithy): Codegen improvements

- Adds further override capabilities for shapes
- Aligns `@default` and `@required` behavior more closely to the spec
- Only generates `_init` if necessary

* chore(aft): Don't generate V1 models for `custom` service

Since this tests very specific V2 features, the V1 transformation doesn't provide any value and prevents certain usages.

* test(goldens): Add default value tests

* fix(aft): SDK models are V2

* chore(repo): Regenerate SDK

* chore(repo): Clean up
dnys1 added a commit that referenced this pull request Jul 28, 2023
* chore(smithy): Codegen improvements

- Adds further override capabilities for shapes
- Aligns `@default` and `@required` behavior more closely to the spec
- Only generates `_init` if necessary

* chore(aft): Don't generate V1 models for `custom` service

Since this tests very specific V2 features, the V1 transformation doesn't provide any value and prevents certain usages.

* test(goldens): Add default value tests

* fix(aft): SDK models are V2

* chore(repo): Regenerate SDK

* chore(repo): Clean up
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:squash PR should be submitted with a sqaush commit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants