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: update lerna to v7 and migrate to npm workspace #1636

Closed

Conversation

chigia001
Copy link
Contributor

@chigia001 chigia001 commented Aug 13, 2023

Which problem is this PR solving?

Short description of the changes

  • Add "workspaces" definition to root package.json
  • Remove the "prepare" script in each workspace's package.json
  • Clean up gts as it does not use anymore; I just happened to see this because its part of the lerna bootstrap arguments
  • Fix any hoist issue that occurred with the new setup:
    • zone.js => instrumentation-user-interaction depends on zone.js's type, but the way the type is defined in zone.js is generated, it can't be used with the import statement. We must use /// <reference syntax so that we don't need to fix the path to zone.js/dist/zone.js.d.ts
  • Update GH's action cache using setup-node
    • This will cache the ~/.npm instead of node_modules folder and won't cache any generated package-lock.json
      • this previous setup might create a cache lock file that we unable to replicate on local machine
      • as we also don't use yarn bootstrap anymore, the previous best practice for node lerna cache not apply for this case
  • Remove lerna:link in .tav.yml file

There might be some hoist issues in the future, and the current npmcli script doesn’t provide enough support for us for more complex hoisting like what lerna used to provide.

Blocker

  • NPM's workspace hoisting issue
    • Hoisting issue in example module
      • Should example module use latest code?
      • Mongodb's example module is picking up mongodb@v4 from root folder => this cause mongo's example project failt to compule
    • Should we move to pnpm or Yarn if NPM's hoisting can't be solve?
      • How to handle test-all-version?
      • Benchmark/toolchain?
      • Maintainer consensus?

Next Step

As lerna@7 is just provide some additional feature on top of nx, ee might consider to include nx.json to increase caching between task and apply for nx-cloud. Nx-cloud should be free for OSS project like opentelemetry-js-contrib. Those change might also fix issue with #1473

@@ -4,6 +4,7 @@ on:
push:
branches:
- main
- feat-lerna-bootstrap-deprecate
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will remove this later

@@ -5,6 +5,7 @@ on:
- "main"
- "release/**"
- "release-please/**"
- feat-lerna-bootstrap-deprecate
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will remove this later

@codecov
Copy link

codecov bot commented Aug 13, 2023

Codecov Report

Merging #1636 (45ade57) into main (825b5a8) will decrease coverage by 0.40%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1636      +/-   ##
==========================================
- Coverage   91.78%   91.38%   -0.40%     
==========================================
  Files         139      129      -10     
  Lines        7110     6387     -723     
  Branches     1426     1276     -150     
==========================================
- Hits         6526     5837     -689     
+ Misses        584      550      -34     
Files Changed Coverage Δ
...opentelemetry-instrumentation-fastify/src/utils.ts 87.50% <ø> (ø)

... and 10 files with indirect coverage changes

@chigia001 chigia001 force-pushed the feat-lerna-bootstrap-deprecate branch from 0f18d14 to 5f5f0d3 Compare August 13, 2023 06:54
@github-actions
Copy link
Contributor

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Oct 23, 2023
@mottibec mottibec removed their assignment Nov 2, 2023
Copy link
Contributor

This PR was closed because it has been stale for 14 days with no activity.

@github-actions github-actions bot closed this Nov 13, 2023
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.

Local build differs from CI build Update dependency lerna to v7