-
Notifications
You must be signed in to change notification settings - Fork 68
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
Update to nodejs rules 0.31.1 #87
Update to nodejs rules 0.31.1 #87
Conversation
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the ℹ️ Googlers: Go here for more info. |
df0fdb9
to
d968be6
Compare
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
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 you include something in the commit message that explains the context a bit more, especially around temporarily pointing to your fork?
@jelbourn Should have explained better in the desc. This is not meant to be merged until the commit is changed to point to a rules_nodejs release so the commit to my fork is temporary. PR is here to test against CI and will be updated to rules_nodejs upstream before merging :) |
d968be6
to
e47f9f1
Compare
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the ℹ️ Googlers: Go here for more info. |
e47f9f1
to
d033c0f
Compare
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
Note: this means that rules_sass users will need to update their rules_nodejs version in WORKSPACE along with the next sass release. might want to include that note in the release notes @nex3 |
nodejs_binary entry_point is now a label
d033c0f
to
50493db
Compare
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the ℹ️ Googlers: Go here for more info. |
@@ -34,10 +34,6 @@ rules_sass_dependencies() | |||
# Setup repositories which are needed for the Sass rules. | |||
load("@io_bazel_rules_sass//:defs.bzl", "sass_repositories") | |||
sass_repositories() | |||
|
|||
# Setup the NodeJS toolchain | |||
load("@build_bazel_rules_nodejs//:defs.bzl", "node_repositories") |
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.
node_repositories()
is no longer explicitly since sass_repositories()
calls yarn_install
which will call node_repositories()
if it has not already been called.
@@ -5,9 +5,6 @@ load("//:package.bzl", "rules_sass_dependencies", "rules_sass_dev_dependencies") | |||
rules_sass_dependencies() | |||
rules_sass_dev_dependencies() | |||
|
|||
load("@build_bazel_rules_nodejs//:defs.bzl", "node_repositories") |
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.
node_repositories()
is no longer explicitly since sass_repositories()
calls yarn_install
which will call node_repositories()
if it has not already been called.
|
||
def sass_repositories(): | ||
"""Set up environment for Sass compiler. | ||
""" | ||
|
||
# 0.31.1: entry_point attribute of rules_nodejs is now a label | ||
check_rules_nodejs_version("0.31.1") |
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.
If a rules_sass user is explicitly installing build_bazel_rules_nodejs
in their WORKSPACE then they'll need to update to at least version 0.31.1. If they are not explicitly installing it then rules_sass_dependencies()
will install the correct version transitively.
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
* rules_sass bazelbuild/rules_sass#87 has landed so can switch back to upstream * stardoc commit we were depending on has since been releases so can switch to a release archive of io_bazel
Now that bazelbuild/rules_sass#87 has landed
* update to upstream rules_sass commit now that bazelbuild/rules_sass#87 has landed (+2 squashed * entry_point attributes in rules_nodejs are now labels
* update to upstream rules_sass commit now that bazelbuild/rules_sass#87 has landed (+2 squashed * entry_point attributes in rules_nodejs are now labels * includes temporary post-install patch for @angular/bazel protractor rule which will get removed once that rule is updated in angular repo
* rules_sass bazelbuild/rules_sass#87 has landed so can switch back to upstream * stardoc commits we were depending on have since been releases so can switch to a release archive of io_bazel & io_bazel_skydoc
* update to upstream rules_sass commit now that bazelbuild/rules_sass#87 has landed (+2 squashed * entry_point attributes in rules_nodejs are now labels * includes temporary post-install patch for @angular/bazel protractor rule which will get removed once that rule is updated in angular repo
* rules_sass bazelbuild/rules_sass#87 has landed so can switch back to upstream * stardoc commits we were depending on have since been releases so can switch to a release archive of io_bazel & io_bazel_skydoc
* rules_sass bazelbuild/rules_sass#87 has landed so can switch back to upstream * stardoc commits we were depending on have since been releases so can switch to a release archive of io_bazel & io_bazel_skydoc
nodejs_binary entry_point is now a label. See release notes https://github.com/bazelbuild/rules_nodejs/releases/tag/0.31.0 for more info.
This commit is a pre-req to bazel-contrib/rules_nodejs#777