Skip to content

Do not use getModelWithoutTraitShapes #547

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
merged 1 commit into from
May 25, 2022

Conversation

gosar
Copy link
Contributor

@gosar gosar commented May 24, 2022

The Walker does not traverse relationships create by traits anyways.

Ran yarn generate-clients && yarn test:protocols && yarn generate-clients -s && yarn test:server-protocols in aws-sdk-js-v3 with this change to confirm there is no change to clients. Though there is some rearrangement of code within a file in protocol tests. Here's the diff aws/aws-sdk-js-v3#3633

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@gosar gosar requested a review from mtdowling May 24, 2022 16:28
@gosar gosar requested a review from a team as a code owner May 24, 2022 16:28
@gosar gosar force-pushed the no-getModelWithoutTraitShapes branch from 74124a5 to c48e1d0 Compare May 24, 2022 17:13
@gosar gosar mentioned this pull request May 24, 2022
@@ -180,7 +177,7 @@ void execute() {
// Generate models that are connected to the service being generated.
LOGGER.fine("Walking shapes from " + service.getId() + " to find shapes to generate");
// Walk the tree.
Collection<Shape> shapeSet = new Walker(nonTraits).walkShapes(service);
Collection<Shape> shapeSet = new Walker(model).walkShapes(service);

Model prunedModel = Model.builder().addShapes(shapeSet).build();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary anymore? It was based upon the nonTraits model previously but that's gone, meaning places where this was used can just use model instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still needed. Otherwise, the TopologicalIndex will iterate and visit through more than shapeSet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or I'll change it to use the model and check if shape is present in shapeSet before visiting.

The Walker does not traverse relationships create by traits anyways.
@trivikr trivikr merged commit 078a230 into smithy-lang:main May 25, 2022
@gosar gosar deleted the no-getModelWithoutTraitShapes branch May 26, 2022 00:35
srchase pushed a commit to srchase/smithy-typescript that referenced this pull request Mar 17, 2023
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.

4 participants