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

chore: move karma_web_test to concatjs #2313

Merged
merged 2 commits into from
Dec 2, 2020
Merged

Conversation

alexeagle
Copy link
Collaborator

@alexeagle alexeagle commented Nov 25, 2020

BREAKING CHANGE:

packages/karma:package.bzl is gone, in your WORKSPACE replace

load("//packages/karma:package.bzl", "npm_bazel_karma_dependencies")

npm_bazel_karma_dependencies()

with the equivalent

http_archive(
    name = "io_bazel_rules_webtesting",
    sha256 = "9bb461d5ef08e850025480bab185fd269242d4e533bca75bfb748001ceb343c3",
    urls = ["https://github.com/bazelbuild/rules_webtesting/releases/download/0.3.3/rules_webtesting.tar.gz"],
)

Then in BUILD files replace
load("@npm//@bazel/karma:index.bzl", "karma_web_test_suite")
with
load("@npm//@bazel/concatjs:index.bzl", "karma_web_test_suite")

finally drop npm dependencies on @bazel/karma and depend on @bazel/concatjs instead

@google-cla google-cla bot added the cla: yes label Nov 25, 2020
BREAKING CHANGE:

`packages/karma:package.bzl` is gone, in your WORKSPACE replace

```
load("//packages/karma:package.bzl", "npm_bazel_karma_dependencies")

npm_bazel_karma_dependencies()
```

with the equivalent

```
http_archive(
    name = "io_bazel_rules_webtesting",
    sha256 = "9bb461d5ef08e850025480bab185fd269242d4e533bca75bfb748001ceb343c3",
    urls = ["https://github.com/bazelbuild/rules_webtesting/releases/download/0.3.3/rules_webtesting.tar.gz"],
)
```

Then in BUILD files replace
`load("@npm//@bazel/karma:index.bzl", "karma_web_test_suite")`
with
`load("@npm//@bazel/concatjs:index.bzl", "concatjs_web_test_suite")`

finally drop npm dependencies on `@bazel/karma` and depend on `@bazel/concatjs` instead
@jbedard
Copy link
Collaborator

jbedard commented Nov 26, 2020

Do we not want to keep "karma" in the name somewhere since these rules are karma specific? It's also a bit odd having @bazel/concatjs depend on karma, is it not? :/

@alexeagle
Copy link
Collaborator Author

@jbedard I think we discussed this plan in a meeting. I agree with those points but do you have an alternative proposal?

@jbedard
Copy link
Collaborator

jbedard commented Nov 26, 2020

No I don't, I guess we should just think of this package as "old g3 concatjs stuff", karma included, instead of a generic concatjs package. That's probably what we already decided though...

@alexeagle
Copy link
Collaborator Author

Decided to move but not rename the rule

@alexeagle alexeagle merged commit 252b8e5 into bazel-contrib:3.x Dec 2, 2020
@mattem mattem linked an issue Dec 8, 2020 that may be closed by this pull request
@cfife-btig
Copy link

As someone who is just digging around and trying to get karma_web_test to work with my current project, I would honestly love to know why Karma got sunk in with concatjs. It just seems a bit unintuitive and it sounds like it was resolved in an offline meeting. Are the Karma rules meant to be standalone or are they supposed to be used with Concatjs?

@alexeagle
Copy link
Collaborator Author

The karma implementation here is indeed coupled with concatjs as the bundler. You could use the vanilla karma js tool under Bazel without any custom rules.

@cfife-btig
Copy link

Thank you, that's actually really helpful. I was starting to think the same but it's nice to confirm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move karma_web_test rule to concatjs package
4 participants