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

fix: support node_version_from_nvmrc with bzlmod #3730

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

gregmagolan
Copy link
Collaborator

@gregmagolan gregmagolan commented Apr 2, 2024

Add new node_version_from_nvmrc attribute to bzlmod and deprecate old naming of use_nvmrc used in WORKSPACE. node_version_from_nvmrc naming follows the convention from rules_ts where a similar attribute is named ts_version_from and is more accurate and self-documenting.

@gregmagolan gregmagolan force-pushed the support_bzlmod_use_nvmrc branch 4 times, most recently from bb324ad to 11437b8 Compare April 2, 2024 06:17
@gregmagolan gregmagolan changed the title fix: support use_nvmrc with bzlmod fix: support node_version_from_nvmrc with bzlmod Apr 2, 2024
@gregmagolan gregmagolan force-pushed the support_bzlmod_use_nvmrc branch from 11437b8 to f68905e Compare April 2, 2024 06:21
@jbedard
Copy link
Collaborator

jbedard commented Apr 2, 2024

Can we reduce the diff to just the core change? I'm not sure what half the diff is from...

@gregmagolan
Copy link
Collaborator Author

gregmagolan commented Apr 2, 2024

Can we reduce the diff to just the core change? I'm not sure what half the diff is from...

I explained in the PR summary. The corresponding change to adding node_version_from_nvmrc to bzlmod is deprecating the use_nvmrc for WORKSPACE and auto-converting that usage to node_version_from_nvmrc. That keeps this from being a breaking change. Just adding use_nvmrc to bzlmod would make it more work to then make the rename non-breaking for bzlmod.

@gregmagolan gregmagolan force-pushed the support_bzlmod_use_nvmrc branch from f68905e to 4ffbafc Compare April 2, 2024 13:12
@gregmagolan gregmagolan force-pushed the support_bzlmod_use_nvmrc branch from 4ffbafc to 570ab75 Compare April 2, 2024 13:37
@gregmagolan gregmagolan merged commit 8adb4ef into main Apr 2, 2024
4 checks passed
@gregmagolan gregmagolan deleted the support_bzlmod_use_nvmrc branch April 2, 2024 13:57
@jbedard
Copy link
Collaborator

jbedard commented Apr 2, 2024

@gregmagolan I understand what the change is, that's simple, I don't understand why the diff on the PR needs to be so big though?

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

Successfully merging this pull request may close these issues.

3 participants