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.

Sign up for GitHub

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

Jump to bottom

Downstream bazel support #13836

Merged
merged 2 commits into from
Nov 2, 2018
Merged

Downstream bazel support #13836

merged 2 commits into from
Nov 2, 2018

Conversation

gregmagolan
Copy link
Contributor

@gregmagolan gregmagolan commented Oct 26, 2018 •
edited

No description provided.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 26, 2018
@devversion devversion added in progress This issue is currently in progress pr: merge safe target: patch This PR is targeted for the next patch release labels Oct 27, 2018
@ngbot
Copy link

ngbot bot commented Oct 29, 2018

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

@gregmagolan gregmagolan changed the title [WIP] Downstream bazel support Downstream bazel support Oct 30, 2018
@gregmagolan gregmagolan mentioned this pull request Oct 30, 2018
Merged
devversion
devversion approved these changes Oct 30, 2018
View reviewed changes
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

Nice work! just a few minor comments. let me know if you are done.

WORKSPACE Show resolved Hide resolved
index.bzl Outdated Show resolved Hide resolved
src/lib/input/_input.scss Outdated Show resolved Hide resolved
tools/mat_setup_workspace.bzl Outdated Show resolved Hide resolved
tools/mat_setup_workspace.bzl Outdated Show resolved Hide resolved
@devversion devversion added pr: lgtm and removed in progress This issue is currently in progress labels Oct 30, 2018
jelbourn
jelbourn reviewed Oct 30, 2018
View reviewed changes
WORKSPACE Outdated Show resolved Hide resolved
tools/mat_setup_workspace.bzl Outdated Show resolved Hide resolved
tools/mat_setup_workspace.bzl Outdated Show resolved Hide resolved
src/lib/input/_input.scss Outdated Show resolved Hide resolved
@gregmagolan
Copy link
Contributor Author

gregmagolan commented Oct 30, 2018 •
edited

Okay. Good to go. Lets wait until angular/angular#26837 lands and update the repo to the release today with that included since that PR will be needed for the downstream Bazel build.

edit: DONE

@gregmagolan
Copy link
Contributor Author

Also, looks like the e2e_tests are failing are well so that needs to get resolved as well.

@jelbourn
Copy link
Member

Ack- ping me when this is ready to go

@devversion devversion added blocked This issue is blocked by some external factor, such as a prerequisite PR and removed pr: lgtm labels Oct 30, 2018
devversion
devversion approved these changes Oct 31, 2018
View reviewed changes
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM

index.bzl Outdated Show resolved Hide resolved
index.bzl Outdated Show resolved Hide resolved
@devversion devversion added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed blocked This issue is blocked by some external factor, such as a prerequisite PR labels Oct 31, 2018
@jelbourn
Copy link
Member

jelbourn commented Nov 1, 2018

This is not merge safe because it touches library code

@jelbourn jelbourn added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Nov 1, 2018
@jelbourn
Copy link
Member

jelbourn commented Nov 1, 2018

Caretaker note: this will require a BUILD update internally

jelbourn
jelbourn approved these changes Nov 1, 2018
View reviewed changes
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@devversion
Copy link
Member

Ah yeah, I've had merge safe applied before the form-field stylesheet change.

@jelbourn
Copy link
Member

jelbourn commented Nov 2, 2018

@gregmagolan could you rebase?

@gregmagolan
Copy link
Contributor Author

@jelbourn Rebased. Good to go.

@jelbourn jelbourn merged commit 150c964 into angular:master Nov 2, 2018
@devversion devversion mentioned this pull request Nov 2, 2018
Merged
atscott pushed a commit to atscott/components that referenced this pull request Nov 5, 2018
@gregmagolan @atscott
fix: bazel support for downstream apps (angular#13836)
7e3e574
jelbourn pushed a commit that referenced this pull request Nov 6, 2018
@gregmagolan @jelbourn
fix: bazel support for downstream apps (#13836)
61dbd26
@gregmagolan gregmagolan mentioned this pull request Nov 8, 2018
Closed
@angular-automatic-lock-bot
Copy link

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 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jelbourn jelbourn jelbourn approved these changes

@devversion devversion devversion approved these changes

@andrewseguin andrewseguin Awaiting requested review from andrewseguin

@crisbeto crisbeto Awaiting requested review from crisbeto

@mmalerba mmalerba Awaiting requested review from mmalerba

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@gregmagolan @jelbourn @devversion @googlebot