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

chore: improve npm release scripts #4972

Merged
merged 3 commits into from
Apr 3, 2020
Merged

Conversation

raymondfeng
Copy link
Contributor

@raymondfeng raymondfeng commented Mar 25, 2020

Improve npm build scripts:

  • Add update-peer-deps to maintain peer dependencies during the release process
  • Move update of package-lock.json out of version phase to ensure testing against latest versions and remove duplicate update.
  • Improve rebuild-package-locks to accept optional lerna scope to rebuild matched packages only.

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

@raymondfeng raymondfeng mentioned this pull request Mar 25, 2020
7 tasks
@raymondfeng raymondfeng added the Internal Tooling Issues related to our tooling and monorepo infrastructore label Mar 27, 2020
Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

+1 to update peer dependencies as part of our version release process 👍

I'd like to discuss few aspects of the proposal, see my comments below.

bin/rebuild-package-locks.js Show resolved Hide resolved
const pkgVersion = lbModuleVersions[d];
if (pkgVersion) {
// Update the version range to be `^<major>.0.0`
pkgJson.peerDependencies[d] = `^${semver.major(pkgVersion)}.0.0`;
Copy link
Member

Choose a reason for hiding this comment

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

Should we use the actual pkgVersion, since that's the version we are testing in our CI? If we use MAJOR.0.0, then it's possible to create a situation where the peer dep spec is too wide, because the extension depends on features introduced in semver-minor version after MAJOR.0.0.

Another aspect to consider: removing a version from peerDependencies range is a breaking change, similarly to how dropping support for a Node.js major version is a breaking change too.

It would be great if lerna was handling these complexities automatically for us, see the discussion in lerna/lerna#955 and lerna/lerna#1018

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's be conservative to use the ^<pkgVersion> for peer dependencies as a release is cut.

@raymondfeng
Copy link
Contributor Author

@bajtos PTAL.

@raymondfeng raymondfeng requested a review from bajtos March 30, 2020 16:39
@dhmlau dhmlau added this to the April 2020 milestone Apr 2, 2020
bin/rebuild-package-locks.js Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal Tooling Issues related to our tooling and monorepo infrastructore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants