Skip to content

feat: move clipboard module into cdk #17272

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

Merged
merged 1 commit into from
Oct 4, 2019

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Oct 2, 2019

Moves the clipboard module into CDK stable, sets up the API goldens, adjusts some APIs to be more consistent and sets up a live example.

@crisbeto crisbeto added P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful target: major This PR is targeted for the next major release labels Oct 2, 2019
@crisbeto crisbeto requested review from jelbourn and a team as code owners October 2, 2019 14:33
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 2, 2019
@crisbeto crisbeto force-pushed the clipboard-module branch 3 times, most recently from 0389035 to 6510231 Compare October 2, 2019 17:43
@mmalerba mmalerba added this to the 9.0.0 milestone Oct 2, 2019
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 on the Bazel part.

Moves the `clipboard` module into CDK stable, sets up the API goldens, adjusts some APIs to be more consistent and sets up a live example.
@crisbeto
Copy link
Member Author

crisbeto commented Oct 3, 2019

I've addressed the feedback @jelbourn.

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

styleUrls: ['cdk-clipboard-overview-example.css'],
})
export class CdkClipboardOverviewExample {
value = `Did you ever hear the tragedy of Darth Plagueis The Wise? I thought not. It's not ` +
Copy link
Member

Choose a reason for hiding this comment

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

A surprise for sure, but a welcome one.

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Oct 3, 2019
@jelbourn
Copy link
Member

jelbourn commented Oct 3, 2019

Caretaker note: this will require updating build rules internally

@mmalerba mmalerba merged commit 473d4c6 into angular:master Oct 4, 2019
@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 Nov 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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 P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants