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: add polymorphDafny and polymorphDotnet tasks to Smithy Build Plugin-based SQS TestModel #395

Merged
merged 25 commits into from
Jun 11, 2024

Conversation

kessplas
Copy link
Contributor

@kessplas kessplas commented May 29, 2024

Issue #, if available: #356 (related)

Description of changes: This PR allows the Smithy Build Plugin-based SQS TestModel to be built and structured similarly to the Makefile/CLI based models used elsewhere. This enables some Smithy features which are not supported by the CLI, such as Projections.

Instead of running make polymorph_dafny and make polymorph_dotnet, run ./gradlew polymorphDafny and ./gradlew polymorphDotnet to generate types for the respective languages. A Makefile is included, as it is needed to run make transpile_net and make test_net, which work as expected when the above commands are executed.

NOTE: These changes are not tested in CI. That may be done as part of this PR, or later, depending on what reviewers prefer.

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

@kessplas kessplas marked this pull request as ready for review May 29, 2024 21:31
@kessplas kessplas requested a review from a team as a code owner May 29, 2024 21:31
@@ -1,5 +1,23 @@
{
"version": "1.0",
"projections": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we make this listQueuesOnly and actually hook it up as the source in the Gradle build via projectionName = "listQueuesOnly" as above? That way we're actually testing the workflow we're claiming to work now, and TestComAmazonawsSqs.dfy should still work AFAICT.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good call!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Only nit is that "listQueuesOnly" is now technically "listQueuesOnlyExceptForAtLeastOneOtherOneBecauseOf#439" :) But it doesn't look like smithy-build.json supports comments. Perhaps we could add a comment to the README of this test model?

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 point, I'll do both.
(JSON in general doesn't support comments, it's kind of a bummer)

@kessplas kessplas requested a review from robin-aws June 10, 2024 22:56
robin-aws
robin-aws previously approved these changes Jun 10, 2024
@kessplas kessplas requested a review from robin-aws June 10, 2024 23:55
@robin-aws robin-aws merged commit b7cec71 into main-1.x Jun 11, 2024
152 checks passed
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