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

feat(upgrade): add package.json update flow to cli #10388

Merged

Conversation

joshblack
Copy link
Contributor

@joshblack joshblack commented Jan 11, 2022

Closes #10190

This PR adds in an MVP for the Upgrade CLI to support various upgrade strategies. Currently, it supports previewing and writing changes made to a workspace's package.json file. In the future, it will also support running migrations for the updates in an upgrade path.

Changelog

New

  • Use esbuild to bundle packages/upgrade into a single executable cli.js
  • Split out our CLI into two distinct areas to help with testability: the core CLI layer and commands
    • The CLI layer relies on manual testing and should change infrequently as a result, it's difficult to e2e CLI tools (as far as I'm aware) that require input into stdin based on questions
    • The commands (in this case commands/upgrade.js) contain all functionality for a command and have integration tests (and can have unit tests)

Changed

  • Update ESLint to ignore the generated cli.js file in packages/upgrade
  • Update Jest to correctly compile .cjs and .mjs files

Removed

  • Remove files that are not needed for the MVP

Testing / Reviewing

  • Take a look at the overall approach, ask questions if you're unsure how it works, give feedback if something feels off in this MVP
  • Verify tests pass as expected (also make sure the tests accurately cover a broad range of cases)
  • Try out the CLI locally by:
    • Pulling down this PR
    • Run cd packages/upgrade
    • Run yarn build to build the CLI
    • Run cd fixtures/sample-project to use our sample project fixture
    • Run ../../bin/carbon-upgrade.js --help to verify the CLI is up-and-running
    • Take a look at the options / capabilities of what's there to see if it makes sense
    • Run ../../bin/carbon-upgrade.js and choose an upgrade path. Verify the CLI does not change the file and shows a preview of the changes
    • Run ../../bin/carbon-upgrade.js -w and choose an upgrade path. Verify the CLI edits the file locally (and discard the changes)

@joshblack joshblack requested a review from a team as a code owner January 11, 2022 23:11
@joshblack joshblack requested review from tw15egan, aledavila, jnm2377, dakahn and tay1orjones and removed request for tw15egan and aledavila January 11, 2022 23:11
@joshblack joshblack changed the title feat(upgrade): add package.json update support to cli feat(upgrade): add package.json update flow to cli Jan 11, 2022
@netlify
Copy link

netlify bot commented Jan 11, 2022

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: b937687

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/61e1e4637300ab000749526e

😎 Browse the preview: https://deploy-preview-10388--carbon-react-next.netlify.app/

@netlify
Copy link

netlify bot commented Jan 11, 2022

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: b937687

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/61e1e4639d39fc0007d7250d

😎 Browse the preview: https://deploy-preview-10388--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Jan 11, 2022

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: b937687

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/61e1e46343170b0009487237

😎 Browse the preview: https://deploy-preview-10388--carbon-components-react.netlify.app

@@ -5,96 +5,112 @@
* LICENSE file in the root directory of this source tree.
*/

'use strict';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the use strict be removed because this file is going through the esbuild process?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah! Definitely

Copy link
Contributor

@jnm2377 jnm2377 left a comment

Choose a reason for hiding this comment

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

🔥 🔥 🔥
tested out and it works great!!!
Screen Shot 2022-01-14 at 11 47 40 AM

Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

🔥 🎉

@kodiakhq kodiakhq bot merged commit c0e02b4 into carbon-design-system:main Jan 14, 2022
@abbeyhrt abbeyhrt mentioned this pull request Jan 27, 2022
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add package update path to Migration CLI
4 participants