Skip to content

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

Closed
adamyi wants to merge 1 commit into
angular:masterfrom
adamyi:bazel_tsconfig_extend
Closed

feat(bazel): add ts_config extending support for ng_module#21883
adamyi wants to merge 1 commit into
angular:masterfrom
adamyi:bazel_tsconfig_extend

Conversation

@adamyi

@adamyi adamyi commented Jan 30, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@adamyi
adamyi force-pushed the bazel_tsconfig_extend branch from 536db42 to e71fe89 Compare January 30, 2018 09:45
@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
alexeagle
alexeagle reviewed Feb 2, 2018
View reviewed changes
Comment thread packages/bazel/src/rules_typescript.bzl Outdated

@alexeagle alexeagle Feb 2, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
alexeagle approved these changes Feb 2, 2018
View reviewed changes

@alexeagle alexeagle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!!

@alexeagle

alexeagle commented Feb 2, 2018

Copy link
Copy Markdown
Contributor

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

laurentlb commented Feb 6, 2018

Copy link
Copy Markdown
Contributor

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

@mhevery mhevery added the area: core Issues related to the framework runtime label Feb 13, 2018
@ngbot

ngbot Bot commented Feb 18, 2018

Copy link
Copy Markdown

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 bazel_tsconfig_extend branch from e71fe89 to 68a033c Compare March 1, 2018 14:24
@alexeagle alexeagle added the action: merge The PR is ready for merge by the caretaker label Mar 1, 2018
@ngbot

ngbot Bot commented Mar 1, 2018

Copy link
Copy Markdown

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.

@alexeagle alexeagle added the target: major This PR is targeted for the next major release label Mar 1, 2018
@kara kara added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels Mar 12, 2018
@ngbot

ngbot Bot commented Apr 2, 2018

Copy link
Copy Markdown

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 bazel_tsconfig_extend branch from 68a033c to de410ba Compare February 25, 2019 17:39
@adamyi
adamyi requested a review from a team February 25, 2019 17:39
@alexeagle
alexeagle force-pushed the bazel_tsconfig_extend branch from de410ba to 61c6d34 Compare February 25, 2019 17:47
@alexeagle
alexeagle force-pushed the bazel_tsconfig_extend branch from 61c6d34 to 2b8cd89 Compare February 25, 2019 18:00
@alexeagle alexeagle added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Feb 25, 2019
@alexeagle

alexeagle commented Feb 25, 2019

Copy link
Copy Markdown
Contributor

presubmit

@mhevery mhevery closed this in edb6c2d Feb 26, 2019
@gregmagolan gregmagolan mentioned this pull request Feb 26, 2019
Closed
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 14, 2019

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@alexeagle alexeagle alexeagle approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@alexeagle alexeagle

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime cla: yes target: major This PR is targeted for the next major release

Projects

None yet

Milestone

needsTriage

Development

Successfully merging this pull request may close these issues.

7 participants

@adamyi @alexeagle @laurentlb @mhevery @jasonaden @kara @googlebot