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: Update linked packages in package-lock.json #1844

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

padaszewski
Copy link

@padaszewski padaszewski commented Feb 6, 2023

Hi, I'm making a start for #1842 as this is very important for me.
I'm not fully into the code and this is just the starting point in my opinion.
Would love if someone could assist on this one!

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Addresses #1842 🦕

@google-cla
Copy link

google-cla bot commented Feb 6, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@conventional-commit-lint-gcf
Copy link

conventional-commit-lint-gcf bot commented Feb 6, 2023

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@product-auto-label product-auto-label bot added the size: xs Pull request size is extra small. label Feb 6, 2023
@padaszewski
Copy link
Author

Updated CLA

@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: xs Pull request size is extra small. labels Feb 7, 2023
@padaszewski
Copy link
Author

This should be the right direction, but I'm still struggling with the test. Any help would be greatly appreciated!

@chingor13 chingor13 marked this pull request as ready for review March 8, 2023 18:16
@chingor13 chingor13 requested review from a team as code owners March 8, 2023 18:16
Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

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

Please add tests to ensure we don't regress.

I'm still struggling with the test

To test, you can run:

  • npm i
  • npm t

If you don't have node available, you can run this in a stock GitHub Codespace (IIRC, node 19 is installed which should suffice).

As for the contents of the test, you can probably:

  1. augment an existing node-workspace test to ensure that it has an update for the package-lock.json file
  2. add a test to the package-lock-json updater to ensure it updates multiple versions

@padaszewski
Copy link
Author

Sorry, I have currently no time to get back to this, could anyone continue on that one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants