-
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
chore: update bazel/create package #2665
Conversation
alexeagle
commented
May 9, 2021
- put deps in a separate file
- don't name WORKSPACE.bazel due to bug
c47d85f
to
da40cdc
Compare
packages/create/index.js
Outdated
|
||
${pkgMgr === 'yarn' ? yarnInstallCmd : npmInstallCmd}`; | ||
|
||
write('WORKSPACE.bazel', workspaceContent); | ||
write('.bazelignore', `node_modules | ||
write('tools/BUILD.bazel', '# No targets in this Bazel package\n') |
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.
nit: "No targets in this Bazel package" makes it seem to me that it's not valid to put targets in the root package, rather than saying that this package is currently empty?
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.
fixed
write('.bazelignore', `node_modules | ||
write('tools/BUILD.bazel', '# No targets in this Bazel package\n') | ||
write('tools/bazel_deps.bzl', bazelDepsContent); | ||
// Don't name it WORKSPACE.bazel since there's a bug with managed_directories |
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.
@alexeagle could you please share more details about this bug? Does it mean that "workspace" file should be used always without .bazel extension, or it affects only bazel/create package?
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.
@gregmagolan is tracking down the bug which hasn't been filed upstream yet. It affects everyone, not just this bazel/create package.
- put deps in a separate file - don't name WORKSPACE.bazel due to bug
- put deps in a separate file - don't name WORKSPACE.bazel due to bug