Skip to content

chore: changes in @aws-cdk/toolkit-lib should trigger CLI release #364

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

iliapolo
Copy link
Contributor

Now that the CLI depends on @aws-cdk/toolkit-lib, it should be added to the releasable commits list.

Also, the transitiveToolkitPackages function, which is used to determine the packages we trigger releases on, did not consider whether the specific toolkit package is actually a dependency of the provided package or not.

This caused incorrect releases. For example:

  • cli-lib-alpha was released when tmp-toolkit-helpers was changed, even though it does not depend on it.

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

@aws-cdk-automation aws-cdk-automation requested a review from a team April 14, 2025 13:33
@github-actions github-actions bot added the p2 label Apr 14, 2025
@iliapolo iliapolo changed the title chore: changes in @aws-cdk/toolkit-lib do not trigger CLI release chore: changes in @aws-cdk/toolkit-lib should trigger CLI release Apr 14, 2025
];

return transitiveFeaturesAndFixes(thisPkg, toolkitPackages.filter(name => name !== thisPkg));
const manifest = require(`${__dirname}/packages/${thisPkg}/package.json`);
Copy link
Contributor

Choose a reason for hiding this comment

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

This will only work after synthesis.

So if we add a new dependency we need to do

npx projen
npx projen

To pick it up. Can't we do it another way?

...Object.keys(manifest.devDependencies ?? {}),
]));

return transitiveFeaturesAndFixes(thisPkg, toolkitPackages.filter(name => name !== thisPkg && deps.includes(name)));
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not really transitive this way, is it?

rix0rrr pushed a commit that referenced this pull request Apr 16, 2025
…380)

Until we sort out #364 properly,
lets at least do this so that toolkit changes trigger a CLI release.

---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants