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

feat(rosetta): Rosetta manages dependencies automatically #3269

Merged
merged 21 commits into from Feb 9, 2022

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Dec 17, 2021

It's quite an annoying bother and pretty brittle to have to set up a full dependency directory to compile examples in.

In CDK, we use the directory of the decdk package to do this, because we have external knowledge that all packages
will be symlinked into this directory.

That mechanism is brittle and revolves around having outside knowledge. Instead, make Rosetta manage the dependency closure itself, when user declare exampleDependencies in package.json:

{
  "jsiiRosetta": {
    "exampleDependencies": {
      "@some-other/package": "^1.2.3",
      "@yet-another/package": "*",
    }
  }
}

It is not necessary to redeclare any of the dependencies and peerDependencies already in package.json: those are automatically included.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@rix0rrr rix0rrr requested a review from a team December 17, 2021 13:43
@rix0rrr rix0rrr self-assigned this Dec 17, 2021
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Dec 17, 2021
@mergify
Copy link
Contributor

mergify bot commented Dec 22, 2021

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.
[Conventional Commits]: https://www.conventionalcommits.org

@mergify
Copy link
Contributor

mergify bot commented Dec 22, 2021

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.
[Conventional Commits]: https://www.conventionalcommits.org

packages/jsii-rosetta/lib/find-utils.ts Outdated Show resolved Hide resolved
packages/jsii-rosetta/lib/find-utils.ts Outdated Show resolved Hide resolved
packages/jsii-rosetta/lib/find-utils.ts Show resolved Hide resolved
packages/jsii-rosetta/lib/find-utils.ts Show resolved Hide resolved
packages/jsii-rosetta/lib/jsii/assemblies.ts Show resolved Hide resolved
packages/jsii-rosetta/lib/snippet-dependencies.ts Outdated Show resolved Hide resolved
packages/jsii-rosetta/lib/snippet-dependencies.ts Outdated Show resolved Hide resolved
packages/jsii-rosetta/lib/snippet-dependencies.ts Outdated Show resolved Hide resolved
packages/jsii-rosetta/lib/snippet.ts Show resolved Hide resolved
packages/jsii-rosetta/lib/util.ts Outdated Show resolved Hide resolved
@rix0rrr rix0rrr requested review from RomainMuller and a team January 13, 2022 13:38
@@ -7,7 +7,7 @@ export async function checkCoverage(assemblyLocations: readonly string[]): Promi
logging.info(`Loading ${assemblyLocations.length} assemblies`);
const assemblies = await loadAssemblies(assemblyLocations, false);

const snippets = Array.from(allTypeScriptSnippets(assemblies, true));
const snippets = Array.from(await allTypeScriptSnippets(assemblies, true));
Copy link
Contributor

Choose a reason for hiding this comment

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

allTypeScriptSnippets returns a promise to an array already...

Suggested change
const snippets = Array.from(await allTypeScriptSnippets(assemblies, true));
const snippets = await allTypeScriptSnippets(assemblies, true);

@mergify
Copy link
Contributor

mergify bot commented Feb 1, 2022

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

@mergify mergify bot added the pr/ready-to-merge This PR is ready to be merged. label Feb 1, 2022
@mergify
Copy link
Contributor

mergify bot commented Feb 1, 2022

Merging (with squash)...

@mergify
Copy link
Contributor

mergify bot commented Feb 9, 2022

Merging (with squash)...

@mergify mergify bot merged commit f0b811b into main Feb 9, 2022
@mergify mergify bot deleted the huijbers/rosetta-own-deps branch February 9, 2022 10:34
@mergify mergify bot removed the pr/ready-to-merge This PR is ready to be merged. label Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants