chore: use non-caret dependency versions #246
Merged
Conversation
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
rix0rrr
had a problem deploying
to
integ-approval
GitHub Actions
Failure
mrgrain
approved these changes
Mar 18, 2025
|
I'm just worried this will affect the release process, read: bump and unbump and if the repo is considered unchanged afterwards. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #246 +/- ##
==========================================
- Coverage 85.03% 85.03% -0.01%
==========================================
Files 206 218 +12
Lines 35202 36530 +1328
Branches 4533 4569 +36
==========================================
+ Hits 29934 31062 +1128
- Misses 5116 5318 +202
+ Partials 152 150 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mrgrain
reviewed
Mar 18, 2025
.projenrc.ts
Outdated
Comment on lines
1062
to
1063
| cloudAssemblySchema.customizeReference({ exactVersion: true }), | ||
| cloudFormationDiff.customizeReference({ exactVersion: true }), |
There was a problem hiding this comment.
That's not right. Toolkit Library intentionally uses dependency ranges. In fact it should probably be wider and declare the actual minimal version and not just ^current.
There was a problem hiding this comment.
I'll happily roll these back to carets, but I don't think it makes sense to use lower versions than current.
- For a fresh install:
^xwill use the highest possible version greater thanx. It doesn't matter if we write^xor^y, ifyis available it will be installed. - For an
npm upgrade: by default it will upgrade everything, so same scenario as fresh install. - For an
npm upgrade toolkit-only: the use case is that a user wants a new version of the toolkit, but explicitly wants to hold back versions of dependency packages? Do we think that is realistic?
rix0rrr
force-pushed
the
huijbers/exact-deps
branch
from
f995133 to
32c0b68
Compare
rix0rrr
had a problem deploying
to
integ-approval
GitHub Actions
Failure
aws-cdk-automation
had a problem deploying
to
integ-approval
GitHub Actions
Failure
rix0rrr
force-pushed
the
huijbers/exact-deps
branch
from
15c1395 to
6a01f80
Compare
aws-cdk-automation
pushed a commit
that referenced
this pull request
Mar 19, 2025
(Stacked PR on top of #246) --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
aws-cdk-automation
had a problem deploying
to
integ-approval
GitHub Actions
Failure
rix0rrr
force-pushed
the
huijbers/exact-deps
branch
from
a792b5c to
6a01f80
Compare
rix0rrr
force-pushed
the
huijbers/exact-deps
branch
from
6a01f80 to
794c0d8
Compare
rix0rrr
force-pushed
the
huijbers/exact-deps
branch
from
794c0d8 to
f9df8bf
Compare
rix0rrr
had a problem deploying
to
integ-approval
GitHub Actions
Failure
Monorepo dependencies were using carets `^`; for most of these it didn't matter because the resulting artifact was bundled anyway, but for a couple it did matter: * `cdk -> aws-cdk`: should use the exact same version, otherwise running `npx [email protected]` may accidentally execute `[email protected]`.
rix0rrr
force-pushed
the
huijbers/exact-deps
branch
from
f9df8bf to
4ea903f
Compare
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 20, 2025
This adds in the code for `@aws-cdk/integ-runner`, which is being removed from the original `aws-cdk` repository here: aws/aws-cdk#33835 Process: * The removal PR should be merged first * Then this should be merged * Then we need to find [the latest released version](https://www.npmjs.com/package/@aws-cdk/integ-runner) and add a tag named `@aws-cdk/[email protected]` (or whatever the latest version is) on the parent commit of this commit. This proposed to drop the `-alpha` release tag of this package, which seems fine. We've been running this for years, it hasn't changed much, and future updates will be easier to consume if we drop the prerelease tags. (Stacked PR on top of #246) --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license --------- Signed-off-by: github-actions <[email protected]> Co-authored-by: github-actions <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Monorepo dependencies were using carets
^; for most of these it didn't matter because the resulting artifact was bundled anyway, but for at least one it does matter:cdk -> aws-cdk: should use the exact same version, otherwise runningnpx [email protected]may accidentally execute[email protected].Also making the
cloud-assembly-schemareferences exact; probably doesn't matter since we're bundling most of these but it does show the intention of shipping exactly one version.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license