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

refactor: rename node_package to link_node_package #75

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

gregmagolan
Copy link
Member

@gregmagolan gregmagolan commented Apr 29, 2022

and node_modules() macro to link_node_packages() as this is more accurate and is future proof for linking Plug'n'Play which doesn't have a node_modules folder

@gregmagolan gregmagolan force-pushed the rename_rules branch 2 times, most recently from bb1bdf4 to 68ac86a Compare April 29, 2022 10:15
@gregmagolan gregmagolan marked this pull request as ready for review April 29, 2022 10:44
@gregmagolan
Copy link
Member Author

This was meant to be a draft still as its based off of #48. Going to land that first and then rebase.

@alexeagle
Copy link
Member

looks like a stack on #48 - I'll review after the rebase

Copy link
Member

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

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

love it. I think keeping the term "link" will help users understand the transition here as well.

@@ -1,3 +1,3 @@
docs/*.md linguist-generated=true
pnpm-lock.yaml linguist-generated=true
e2e/node_package/rules_foo/npm_repositories.bzl linguist-generated=true
e2e/link_node_package/rules_foo/npm_repositories.bzl linguist-generated=true
Copy link
Member

Choose a reason for hiding this comment

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

wonder if we should use a filename convention for vendored repositories.bzl files

Copy link
Member Author

Choose a reason for hiding this comment

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

its too bad github doesn't recognize the @generated on the first line

"@generated by @aspect_rules_js//js/private:translate_pnpm_lock.bzl from pnpm lock file @rules_foo@rules_foo//foo:pnpm-lock.yaml"

@gregmagolan gregmagolan merged commit 5c9d4fc into aspect-build:main Apr 29, 2022
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.

2 participants