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

feat(bazel): add ts_config extending support for ng_module #21883

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
7 participants
@adamyi
Copy link
Contributor

adamyi commented Jan 30, 2018

Support using the extending feature of ts_config in rules_typescript (it didn't add the father ts_config file to the sandbox in the past).

(This is a step towards supporting importing material2 as a Bazel dependency as well.)

/cc @alexeagle @pshields

@googlebot googlebot added the cla: yes label Jan 30, 2018

@adamyi adamyi force-pushed the adamyi:bazel_tsconfig_extend branch from 536db42 to e71fe89 Jan 30, 2018

@adamyi adamyi changed the title build(bazel): add ts_config extending support for ng_module feat(bazel): add ts_config extending support for ng_module Jan 30, 2018

@@ -17,10 +17,15 @@ load("@build_bazel_rules_typescript//internal:common/json_marshal.bzl",
_json_marshal = "json_marshal",
)

load("@build_bazel_rules_typescript//internal:ts_config.bzl",

This comment has been minimized.

Copy link
@alexeagle

alexeagle Feb 2, 2018

Contributor

I'll have to work around - this file is different internally at google to reference rules in their monorepo roots so some care will be needed to merge it.

@alexeagle
Copy link
Contributor

alexeagle left a comment

Thank you!!

@alexeagle

This comment has been minimized.

Copy link
Contributor

alexeagle commented Feb 2, 2018

Gah, the skylint error here is a false positive - it doesn't handle the case that a provider is declared in one file and loaded in another. @laurentlb is it yours?

@laurentlb

This comment has been minimized.

Copy link

laurentlb commented Feb 6, 2018

Yes, it looks like an issue with the linter. We'll take a look

@ngbot

This comment has been minimized.

Copy link

ngbot bot commented Feb 18, 2018

Hi @adamyi! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@alexeagle alexeagle self-assigned this Mar 1, 2018

@alexeagle alexeagle force-pushed the adamyi:bazel_tsconfig_extend branch from e71fe89 to 68a033c Mar 1, 2018

@ngbot

This comment has been minimized.

Copy link

ngbot bot commented Mar 1, 2018

I see that you just added the PR action: merge label, but the following checks are still failing:
    failure missing required label: "PR target: *"
    pending status "google3" is pending
    pending status "ci/circleci: lint" is pending
    pending status "ci/circleci: build" is pending
    pending status "continuous-integration/travis-ci/pr" is pending

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@ngbot

This comment has been minimized.

Copy link

ngbot bot commented Apr 2, 2018

Hi @adamyi! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@jasonaden jasonaden added this to the needsTriage milestone Jan 29, 2019

@alexeagle alexeagle force-pushed the adamyi:bazel_tsconfig_extend branch from 68a033c to de410ba Feb 25, 2019

@adamyi adamyi requested a review from angular/fw-dev-infra as a code owner Feb 25, 2019

@alexeagle alexeagle force-pushed the adamyi:bazel_tsconfig_extend branch from de410ba to 61c6d34 Feb 25, 2019

@alexeagle alexeagle force-pushed the adamyi:bazel_tsconfig_extend branch from 61c6d34 to 2b8cd89 Feb 25, 2019

@alexeagle

This comment has been minimized.

Copy link
Contributor

alexeagle commented Feb 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.