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

Bump lockfile versions for Beachball #41

Closed
rajsite opened this issue Jul 23, 2021 · 6 comments
Closed

Bump lockfile versions for Beachball #41

rajsite opened this issue Jul 23, 2021 · 6 comments

Comments

@rajsite
Copy link
Member

rajsite commented Jul 23, 2021

Beachball will bump package.json versions on publish but will not result in package-lock.json changes.
This causes local npm installs to result in dirty package-locks which get included in PR reviews. This increases the chance of unnecessary merge conflicts in the package-lock.json.
Beachball issue: microsoft/beachball#525 (comment)

@rajsite
Copy link
Member Author

rajsite commented Jul 30, 2021

Fixed with main.yml and package.json changes in compare range: 0b923ee...cfafcd4

@rajsite
Copy link
Member Author

rajsite commented Jul 30, 2021

@jattasNI want to do a quick sanity check of these commits that were direct to master (to sanity check build) before closing this issue?

edit: nvm, spoke too soon, looks like it broke something: https://github.com/ni/nimble/runs/3204626431

@rajsite
Copy link
Member Author

rajsite commented Jul 30, 2021

Reverted the changes and commented on the linked fast issue. Another thing to try is letting beachball publish and push as-is, then sync main to pull in the beachball changes, do the npm i --package-lock-only, and push that.

@rajsite
Copy link
Member Author

rajsite commented Aug 6, 2021

@jattasNI Went on a test spree and got it working with a change in main.yml.

Could you take a look and see if you have any feedback? If beachball adds native npm workspaces support in the future then we can remove the workaround.

@jattasNI
Copy link
Contributor

jattasNI commented Aug 6, 2021

@rajsite your changes to publish package-lock.json updates look correct to me. I was slightly worried about how long the step would take, but looks like it's only 12s and it doesn't run on PR builds, so that's not a problem.

I am surprised beachball isn't taking that issue more seriously--it seems like a major gap to me but the issue isn't getting much discussion. If you can think of a way to nudge them on it (file a new issue? tag the maintainers?) that might be a good step.

@rajsite
Copy link
Member Author

rajsite commented Aug 6, 2021

Looks like fast is considering switching to npm 7 so I threw in a comment describing our use case there: microsoft/fast#4737 (comment)
Also updated the beachball thread with this workaround: microsoft/beachball#525 (comment)

I'll close this issue since we have a form of publishing the lock file working. Hopefully we can remove the workaround in the future.

@rajsite rajsite closed this as completed Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants