chore: make NPM dist tag explicit for all packages #13109
Merged
Conversation
This file contains 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
Currently, our NPM tag strategy is uniform across all packages. This _mostly_ works, but for V2 it makes sense for `aws-cdk-lib` to be published with a dist tag of 'latest' while still publishing all of the other libraries as 'next'. This change is the first supporting change to enable us to have different dist tags for different packages. * Introduces a new pkglint rule that enforces that each package.json has a publishConfig and tag. For v1, this defaults to 'latest'; for v2 it will default to 'next'. These are the correct values, except for `aws-cdk-lib`, which will need to be manually changed to `latest`. * Removes the disttag element from `release.json`. * Changing `check-api-compatibility` to use the dist tags from the individual `package.json` files, rather than from `release.json`. Files to look at (all others are package.json changes): * release.json * scripts/check-api-compatibility.sh * scripts/resolve-version-lib.js * scripts/script-tests/resolve-version.test.js * tools/pkglint/lib/rules.ts Once this is approved, and merged up to V2, the change to flip the aws-cdk-lib tag can be made. Finally, we will update our publishing infrastructure so these new tags are respected. related #13058
|
|
nija-at
suggested changes
Feb 17, 2021
tools/pkglint/lib/rules.ts
Outdated
Comment on lines
58
to
67
| if (!pkg.json.publishConfig?.tag) { | ||
| pkg.report({ | ||
| ruleName: this.name, | ||
| message: 'publishConfig.tag is required', | ||
| fix: (() => { | ||
| const publishConfig = pkg.json.publishConfig ?? {}; | ||
| publishConfig.tag = defaultPublishTag; | ||
| pkg.json.publishConfig = publishConfig; | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Don't we need special logic for aws-cdk-lib, i.e., latest instead of next?
There was a problem hiding this comment.
We could encode it here, but I was planning on just making the change in the aws-cdk-lib package itself. The auto-fix is here to automatically adjust packages where the tag is missing entirely, not encode the special one-off rule for the single exception (IMO).
There was a problem hiding this comment.
I think it's better to have all of the logic encoded here. We should also err/fix if the tag is incorrect, not just for missing.
nija-at
suggested changes
Feb 17, 2021
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
nija-at
approved these changes
Feb 17, 2021
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
NovakGu
pushed a commit
to NovakGu/aws-cdk
that referenced
this pull request
Feb 18, 2021
Currently, our NPM tag strategy is uniform across all packages. This _mostly_ works, but for V2 it makes sense for `aws-cdk-lib` to be published with a dist tag of 'latest' while still publishing all of the other libraries as 'next'. This change is the first supporting change to enable us to have different dist tags for different packages. * Introduces a new pkglint rule that enforces that each package.json has a publishConfig and tag. For v1, this defaults to 'latest'; for v2 it will default to 'next'. These are the correct values, except for `aws-cdk-lib`, which will need to be manually changed to `latest`. * Removes the disttag element from `release.json`. * Changing `check-api-compatibility` to use the dist tags from the individual `package.json` files, rather than from `release.json`. Files to look at (all others are package.json changes): * release.json * scripts/check-api-compatibility.sh * scripts/resolve-version-lib.js * scripts/script-tests/resolve-version.test.js * tools/pkglint/lib/rules.ts Once this is approved, and merged up to V2, the change to flip the aws-cdk-lib tag can be made. Finally, we will update our publishing infrastructure so these new tags are respected. related aws#13058 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
eladb
pushed a commit
that referenced
this pull request
Feb 22, 2021
Currently, our NPM tag strategy is uniform across all packages. This _mostly_ works, but for V2 it makes sense for `aws-cdk-lib` to be published with a dist tag of 'latest' while still publishing all of the other libraries as 'next'. This change is the first supporting change to enable us to have different dist tags for different packages. * Introduces a new pkglint rule that enforces that each package.json has a publishConfig and tag. For v1, this defaults to 'latest'; for v2 it will default to 'next'. These are the correct values, except for `aws-cdk-lib`, which will need to be manually changed to `latest`. * Removes the disttag element from `release.json`. * Changing `check-api-compatibility` to use the dist tags from the individual `package.json` files, rather than from `release.json`. Files to look at (all others are package.json changes): * release.json * scripts/check-api-compatibility.sh * scripts/resolve-version-lib.js * scripts/script-tests/resolve-version.test.js * tools/pkglint/lib/rules.ts Once this is approved, and merged up to V2, the change to flip the aws-cdk-lib tag can be made. Finally, we will update our publishing infrastructure so these new tags are respected. related #13058 ---- *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
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.
Currently, our NPM tag strategy is uniform across all packages. This mostly
works, but for V2 it makes sense for
aws-cdk-libto be published with a disttag of 'latest' while still publishing all of the other libraries as 'next'.
This change is the first supporting change to enable us to have different dist
tags for different packages.
publishConfig and tag. For v1, this defaults to 'latest'; for v2 it will
default to 'next'. These are the correct values, except for
aws-cdk-lib,which will need to be manually changed to
latest.release.json.check-api-compatibilityto use the dist tags from the individualpackage.jsonfiles, rather than fromrelease.json.Files to look at (all others are package.json changes):
Once this is approved, and merged up to V2, the change to flip the aws-cdk-lib
tag can be made. Finally, we will update our publishing infrastructure so these
new tags are respected.
related #13058
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license