Skip to content

test(router): add canDeactivate test for forChild route#36699

Closed
atscott wants to merge 1 commit into
angular:masterfrom
atscott:routertest
Closed

test(router): add canDeactivate test for forChild route#36699
atscott wants to merge 1 commit into
angular:masterfrom
atscott:routertest

Conversation

@atscott

@atscott atscott commented Apr 17, 2020

Copy link
Copy Markdown
Contributor

This PR adds test case to cover a failure that was detected after
merging #36302. That commit will be reverted and will need a new PR that
does not cause this test to fail.

@atscott atscott requested a review from matsko April 17, 2020 23:18
@atscott atscott added area: router action: review The PR is still awaiting reviews from at least one requested reviewer state: blocked target: patch This PR is targeted for the next patch release labels Apr 17, 2020
@ngbot ngbot Bot modified the milestone: needsTriage Apr 17, 2020
@atscott

atscott commented Apr 17, 2020

Copy link
Copy Markdown
Contributor Author

Blocked by #36697

@atscott atscott mentioned this pull request Apr 17, 2020
Closed
14 tasks
@matsko matsko added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 21, 2020
matsko
matsko approved these changes Apr 21, 2020
View reviewed changes

@matsko matsko 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.

LGTM

@atscott atscott 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 Apr 21, 2020
@atscott

atscott commented Apr 21, 2020

Copy link
Copy Markdown
Contributor Author

merge assistance: I'm the only owner of router so pullapprove can't be green.

IgorMinar
IgorMinar approved these changes Apr 21, 2020
View reviewed changes

@IgorMinar IgorMinar 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.

LGTM - @atscott please file an issue against dev-infra to enable code owners to get code changes to their own code approved with just a simple LGTM from anyone on a bigger team (either the fw team, or anyone listed as an approver in the .pullapprove config file). Essentially we want pullapprove to have the same behavior as what we see in google3.

Reviewed-for: global-approvers

@matsko

matsko commented Apr 22, 2020

Copy link
Copy Markdown
Contributor

@atscott please rebase

@atscott
test(router): add canDeactivate test for forChild route
d583a05 
This PR adds test case to cover a failure that was detected after
merging angular#36302. That commit will be reverted and will need a new PR that
does not cause this test to fail.
@matsko matsko closed this in ceb61d1 Apr 23, 2020
matsko pushed a commit that referenced this pull request Apr 23, 2020
@atscott @matsko
test(router): add canDeactivate test for forChild route (#36699)
54883cb 
This PR adds test case to cover a failure that was detected after
merging #36302. That commit will be reverted and will need a new PR that
does not cause this test to fail.

PR Close #36699
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented May 24, 2020

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

Reviewers

2 more reviewers
@matsko matsko matsko approved these changes
@IgorMinar IgorMinar IgorMinar approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: router cla: yes 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

needsTriage

Development

Successfully merging this pull request may close these issues.

4 participants

@atscott @matsko @IgorMinar @googlebot