-
Notifications
You must be signed in to change notification settings - Fork 522
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(builtin): only generate a .tar pkg_npm output when requested #2428
Conversation
@@ -109,6 +118,12 @@ You can use values from the workspace status command using curly braces, for exa | |||
See the section on stamping in the [README](stamping) | |||
""", | |||
), | |||
"tgz": attr.string( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can it be an attr.output
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh wait I see, this attribute isn't actually used? this is just here to document how the macro works? smelly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👃 Yeah, perhaps we refactor it like you suggested to be like ts_project macro, for another PR as that seems an independent change
ef8fe21
to
9c975f5
Compare
internal/pkg_npm/pkg_npm.bzl
Outdated
name = "my_package", | ||
srcs = ["package.json"], | ||
deps = [":my_typescript_lib"], | ||
tgz = "my_package_tgz", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be .tgz
here right?
9c975f5
to
0bd3269
Compare
Adds an optional attribute to the
pkg_npm
macro to generate the.tar
target. When set, the value is used as the filename, which must end in.tgz
. If using this, thepkg_npm
files must include a validpackage.json
file, otherwise npm will error.