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: add root package-lock.json support #2162

Merged
merged 6 commits into from
Jan 2, 2024

Conversation

tada5hi
Copy link
Contributor

@tada5hi tada5hi commented Dec 11, 2023

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)

Fixes #2161
Fixes #1842

@tada5hi tada5hi requested review from a team as code owners December 11, 2023 15:06
Copy link

google-cla bot commented Dec 11, 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.

@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Dec 11, 2023
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.

Rather than explicitly adding a check for a root package-lock.json, would it work if the node-workspace plugin respected the considerAllArtifacts option that other workspace plugins can use?

This option tells the plugin to scan for all "artifacts" (e.g. paths that have a package.json) even if they are undeclared as paths in the release-please-config.json. When building the dependency graph of artifacts that affect each other, the plugin would find the root package.json and create updates for it and its related package-lock.json.

@tada5hi
Copy link
Contributor Author

tada5hi commented Dec 14, 2023

@chingor13 I am unsure, as far as I can tell with my knowledge so far, I would say that this is not the case.

Another problem is that the workspace packages are not (necessarily) part of the root package.json file, but only part of the package-lock.json file. For example, the docs describe a scenario in which the root package is also published and references parts of the workspace packages. However, in most cases this is not the case, as the root package should also be published.

The cargo-workspace plugin follows a similar mechanism as I have planned for the node-workspace plugin, as far as I can tell. However, this assumes that another file in the root directory is already part of the pull request.

@blaine-arcjet
Copy link

@chingor13 I just ran into this same problem. What would it take to get this across the finish line?

@tada5hi
Copy link
Contributor Author

tada5hi commented Dec 20, 2023

@chingor13 ?

@chingor13 chingor13 merged commit 8728d4a into googleapis:main Jan 2, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
3 participants