feat(rosetta): find fixtures based on submodules#3131
Merged
mergify[bot] merged 7 commits intomainfrom Nov 8, 2021
Merged
Conversation
The `rosetta` fixtures directory is assembly-scoped. That means that in the monocdk transform, all Rosetta fixtures from the individual assemblies need to be tossed together into one `rosetta` directory for all of them. Inevitably this leads to conflicts, where the various individual `default.ts-fixture` files compete with each other to be the one, global `default.ts-fixture`. What ends up happening is they end up overwriting each other and the last module to get copied in wins. All other modules' examples fail to compile. Ideally the `ubergen` tool that combines these fixtures would ensure it doesn't clobber existing files, and renames the fixtures to unique names (maybe `s3.default.ts-fixture`). But now, *none* of them are called `default.ts-fixture` and so all snippets require explicit fixture references. Unfortunately, we now need to also automatically add in those references and it's nontrivial to do this automatically. `ubergen` would need to parse and generate both Markdown and Typescript, to add these markers in the right places. Instead, this PR adds the concept of scoped fixtures. If a class (or README) is defined in a submodule, say `aws_s3`, then we will first try `rosetta/aws_s3/default.ts-fixture` before trying `rosetta/default.ts-fixture`. That way, `ubergen` and other tools like it will have an easier time combining default fixtures.
otaviomacedo
reviewed
Nov 8, 2021
| } | ||
|
|
||
| function middle(xs: string[]) { | ||
| if (xs.length <= 2) { |
Contributor
There was a problem hiding this comment.
[minor] Why is this a special case?
Contributor
Author
There was a problem hiding this comment.
You're right, it doesn't need to be.
otaviomacedo
approved these changes
Nov 8, 2021
Contributor
|
The title of this Pull Request does not conform with [Conventional Commits] guidelines. It will need to be adjusted before the PR can be merged. |
Contributor
|
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
Contributor
|
Merging (with squash)... |
Contributor
|
Merging (with squash)... |
mergify bot
pushed a commit
to aws/aws-cdk
that referenced
this pull request
Nov 10, 2021
Currently ubergen bundles all fixtures into the same `rosetta` folder. This means that all fixtures in files called `default.ts-fixture` replace each other as they are found and vie to be the last `default.ts-fixture` standing in the `rosetta` folder. This causes unintended compiliation failures for `yarn rosetta:extract --compile` in projects that rely on ubergen, since they will be copied into the wrong `default.ts-fixture`. The solution here in jsii is to find fixtures based on submodules: aws/jsii#3131. This PR updates ubergen to construct the necessary subfolders in `rosetta` so that the expected submodule fixtures can be found. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO
pushed a commit
to TikiTDO/aws-cdk
that referenced
this pull request
Feb 21, 2022
Currently ubergen bundles all fixtures into the same `rosetta` folder. This means that all fixtures in files called `default.ts-fixture` replace each other as they are found and vie to be the last `default.ts-fixture` standing in the `rosetta` folder. This causes unintended compiliation failures for `yarn rosetta:extract --compile` in projects that rely on ubergen, since they will be copied into the wrong `default.ts-fixture`. The solution here in jsii is to find fixtures based on submodules: aws/jsii#3131. This PR updates ubergen to construct the necessary subfolders in `rosetta` so that the expected submodule fixtures can be found. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
rosettafixtures directory is assembly-scoped. That means that inthe monocdk transform, all Rosetta fixtures from the individual
assemblies need to be tossed together into one
rosettadirectory forall of them.
Inevitably this leads to conflicts, where the various individual
default.ts-fixturefiles compete with each other to be the one,global
default.ts-fixture. What ends up happening is they end upoverwriting each other and the last module to get copied in wins.
All other modules' examples fail to compile.
Ideally the
ubergentool that combines these fixtures would ensureit doesn't clobber existing files, and renames the fixtures to unique
names (maybe
s3.default.ts-fixture). But if it does that, none of them arecalled
default.ts-fixtureand so all snippets require explicitfixture references.
Unfortunately, we now need to also automatically add in those references
and it's nontrivial to do this automatically.
ubergenwould need toparse and generate both Markdown and Typescript, to add these markers
in the right places.
Instead, this PR adds the concept of scoped fixtures. If a class
(or README) is defined in a submodule, say
aws_s3, then we willfirst try
rosetta/aws_s3/default.ts-fixturebefore tryingrosetta/default.ts-fixture.That way,
ubergenand other tools like it will have an easiertime combining default fixtures.
Also renamed
AssemblyFixture=>TestJsiiModuleand made a distinctionbetween the two directories exposed by the object for more clarity on
what it's doing.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.