-
Notifications
You must be signed in to change notification settings - Fork 249
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(aft): Use public models #3727
Conversation
d4b7942
to
e9513c9
Compare
Migrates the `generate sdk` command to use the public `aws-sdk-js-v3` repo for service models instead of the private repo.
Clean up for nullability checks and deserialization logic
e9513c9
to
258667e
Compare
258667e
to
34d1b40
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one non-blocking observation.
.singleWhere( | ||
(service) => | ||
service.expectTrait<ServiceTrait>().endpointPrefix == apiName, | ||
.singleWhere((service) => service.shapeId.namespace == namespace); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool function, didn't know this existed.
for (final operation in includeOperations) { | ||
if (!allOperations.contains(operation)) { | ||
throw Exception( | ||
'Invalid operation specified for $namespace: $operation', | ||
); | ||
includeShapes.addAll(serviceShape.operations.map((op) => op.target)); | ||
} | ||
} | ||
includeShapes.addAll(includeOperations); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could flatten this logic a little bit.
final isMissingOp = includeOperations.every((e) => !allOperations.contains(operation));
if (isMissingOp) {
throw Exception(
'Invalid operation specified for $namespace: $operation',
);
}
includeShapes.addAll(includeOperations);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, started with something like that. This turned out to be cleaner - the problem is getting the element which didn't pass the condition, i.e. operation
in your snippet.
Migrates the
generate sdk
command to use the publicaws-sdk-js-v3
repo for service models instead of the private repo.Fixes some minor codegen issues and regenerates SDK (thus the large PR)