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

refactor: [M3-8632] - Use pnpm as our package manager #11297

Open
wants to merge 68 commits into
base: develop
Choose a base branch
from

Conversation

bnussman-akamai
Copy link
Member

@bnussman-akamai bnussman-akamai commented Nov 20, 2024

Description 📝

  • Uses pnpm as our package manager and monorepo tooling 📦

Breaking Changes ⚠️

  • The up script (yarn up) has been removed because it conflicts with pnpm up. Use pnpm dev instead to start the local development environment
  • pnpm dev no longer installs dependencies for you
    • It is now up to the developer to run pnpm install when they deem nessesary
  • New command pnpm bootstrap is added. It is intended to help developers now that yarn up no longer exists and pnpm dev does less
    • pnpm bootstrap will install dependencies and build packages (api-v4 and validation)

Blocked by 🚧

Todo ☑️

  • Update publish-sdk and publish-validation jobs so our publish step does not fail when resolving workspace protocol dependencies (docs)
  • Ensure devenv is still supported 💻
  • Update documentation 📝

Risk ⚠️

  • Because we are going to use workspace protocol, we need to change how @linode/api-v4 and @linode/validation are published in our Github Action. If something goes wrong, worst case is that the packages won't get published to npm on our next release. Cloud Manager itself will not be affected.

Follow up work 👷

  • Investigate using corepack to ensure developers use the correct package manager
  • yarn to pnpm should be nice upgrade for us but we can still greatly benefit from a tool like NX or Turborepo. We should explore this after we merge this

How to test 🧪

  • Checkout this PR
  • If you use volta, install pnpm (or follow pnpm's guides if you prefer other install methods)
    volta install pnpm@10
  • Now using pnpm, use our clean script to get in a clean state
    pnpm clean
  • "Bootstrap" the repo (this will install dependencies and build packages)
    pnpm  bootstrap
  • Now test anything you can think of that you use on a daily basis
    • pnpm storybook
    • pnpm dev
    • pnpm changeset
    • Anything else you can think of to test...

As an Author, I have considered 🤔

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@bnussman-akamai bnussman-akamai self-assigned this Nov 21, 2024
@bnussman-akamai bnussman-akamai added the Dependencies Pull requests that update a dependency file label Nov 21, 2024
Copy link

This PR is stale because it has been open 15 days with no activity. Please attend to this PR or it will be closed in 5 days

@bnussman-akamai bnussman-akamai changed the title refactor: [M3-8632] - Use pnpm as our package manager refactor: [M3-8632] - Use pnpm as our package manager Jan 30, 2025
@bnussman-akamai bnussman-akamai marked this pull request as ready for review February 7, 2025 15:57
@bnussman-akamai bnussman-akamai requested review from a team as code owners February 7, 2025 15:57

## Serving a production build of Cloud Manager

You can then serve these files however you prefer or use our included local http server.
You can build a prduction bundle of Cloud Manager and serve it locally.

Choose a reason for hiding this comment

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

Suggested change
You can build a prduction bundle of Cloud Manager and serve it locally.
You can build a production bundle of Cloud Manager and serve it locally.

@@ -37,7 +37,7 @@ describe('Link component', () => {
expect(linkElement.tagName).toBe('A');
expect(linkElement).toHaveAttribute('rel', 'noopener noreferrer');
expect(linkElement).toHaveAttribute('target', '_blank');
expect(linkElement.getAttribute('href')).toBe('https://example.com');
expect(linkElement.getAttribute('href')).toBe('https://example.com/');

Choose a reason for hiding this comment

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

Just curious - any reason why the inclusion of pnpm would require this test to change with a trailing slash? In packages/manager/src/components/MainContentBanner.test.tsx as well?

Copy link

@bill-akamai bill-akamai left a comment

Choose a reason for hiding this comment

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

TIL what pnpm is. Nice upgrade @bnussman 👍

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 500 passing tests on test run #59 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing500 Passing2 Skipped103m 1s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants