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

Jump to bottom

fix(router): need to export DefaultRouteReuseStrategy to Router public_api #31575

Closed
wants to merge 1 commit into from
Closed

fix(router): need to export DefaultRouteReuseStrategy to Router public_api #31575

wants to merge 1 commit into from

Conversation

abarghoud
Copy link
Contributor

@abarghoud abarghoud commented Jul 15, 2019 •
edited

fix(router): need to export DefaultRouteReuseStrategy to Router public_api(#30827)

Importing DefaultRouteReuseStrategy is not working currently as it's not exported in index.ts file
Tests exist already.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Not able to import DefaultRouteReuseStrategy from @angular/router

Issue Number: #30827

What is the new behavior?

Ability to import DefaultRouteReuseStrategy

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@abarghoud abarghoud requested a review from as a code owner Jul 15, 2019
@abarghoud abarghoud requested a review from as a code owner Jul 16, 2019
@ngbot ngbot bot added this to the needsTriage milestone Jul 17, 2019
@santoshyadavdev
Copy link
Contributor

@santoshyadavdev santoshyadavdev commented Aug 5, 2019

Hi @abarghoud ,
Can you also stash commits into a single commit.

@googlebot
Copy link

@googlebot googlebot commented Aug 5, 2019

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no and removed cla: yes labels Aug 5, 2019
@abarghoud
Copy link
Contributor Author

@abarghoud abarghoud commented Aug 5, 2019

@googlebot I fixed it.

@googlebot
Copy link

@googlebot googlebot commented Aug 5, 2019

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Aug 5, 2019
@abarghoud
Copy link
Contributor Author

@abarghoud abarghoud commented Aug 5, 2019

@santoshyadav198613 done

@abarghoud abarghoud mentioned this pull request Jun 16, 2020
Closed
14 tasks
@atscott
Copy link
Contributor

@atscott atscott commented Jun 16, 2020

@abarghoud - could you rebase and squash the commits into one?

@atscott atscott linked an issue that may be closed by this pull request Jun 16, 2020
Export DefaultRouteReuseStrategy from router #30827
Closed
@abarghoud
Copy link
Contributor Author

@abarghoud abarghoud commented Jun 16, 2020

@abarghoud - could you rebase and squash the commits into one?

@atscott on it

atscott
atscott requested changes Jun 16, 2020
View changes
packages/router/src/route_reuse_strategy.ts Outdated Show resolved Hide resolved
packages/router/test/integration.spec.ts Outdated Show resolved Hide resolved
@abarghoud abarghoud force-pushed the export-DefaultRouteReuseStrategy branch 4 times, most recently from 6d727c3 to c939bb0 Compare Jun 16, 2020
@abarghoud abarghoud requested review from atscott and removed request for Jun 16, 2020
@abarghoud
Copy link
Contributor Author

@abarghoud abarghoud commented Jun 16, 2020 •
edited

@atscott rebase and squash done.

I see there still are merge conflicts, but i don't know as when I do git rebase master -i I am getting: branch is up to date.

Comments took into account.

atscott
atscott approved these changes Jun 16, 2020
View changes
Copy link
Contributor

@atscott atscott left a comment

reviewed-for: fw-router

packages/router/src/route_reuse_strategy.ts Outdated Show resolved Hide resolved
packages/router/src/route_reuse_strategy.ts Outdated Show resolved Hide resolved
packages/router/src/route_reuse_strategy.ts Outdated Show resolved Hide resolved
@atscott atscott force-pushed the export-DefaultRouteReuseStrategy branch from c939bb0 to 31dc376 Compare Jun 16, 2020
@pullapprove pullapprove bot requested review from alxhub and IgorMinar Jun 16, 2020
@pullapprove pullapprove bot requested a review from atscott Aug 10, 2020
atscott
atscott approved these changes Aug 10, 2020
View changes
Copy link
Contributor

@atscott atscott left a comment

reviewed-for: public-api

@mary-poppins
Copy link

@mary-poppins mary-poppins commented Aug 10, 2020

You can preview fb22e26 at https://pr31575-fb22e26.ngbuilds.io/.

AndrewKushnir
AndrewKushnir approved these changes Aug 10, 2020
View changes
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

LGTM, just left one quick comment.

Reviewed-for: public-api

packages/router/src/route_reuse_strategy.ts Outdated Show resolved Hide resolved
@abarghoud abarghoud force-pushed the export-DefaultRouteReuseStrategy branch from fb22e26 to 0b3b467 Compare Aug 11, 2020
@mary-poppins
Copy link

@mary-poppins mary-poppins commented Aug 11, 2020

You can preview 0b3b467 at https://pr31575-0b3b467.ngbuilds.io/.

@abarghoud abarghoud force-pushed the export-DefaultRouteReuseStrategy branch 2 times, most recently from b4b1133 to 3005050 Compare Aug 11, 2020
@mary-poppins
Copy link

@mary-poppins mary-poppins commented Aug 11, 2020

You can preview 3005050 at https://pr31575-3005050.ngbuilds.io/.

@abarghoud abarghoud force-pushed the export-DefaultRouteReuseStrategy branch from 3005050 to f342b71 Compare Aug 11, 2020
@mary-poppins
Copy link

@mary-poppins mary-poppins commented Aug 11, 2020

You can preview f342b71 at https://pr31575-f342b71.ngbuilds.io/.

@abarghoud
Copy link
Contributor Author

@abarghoud abarghoud commented Aug 11, 2020

@atscott It's good for requested changes

alxhub
alxhub approved these changes Aug 11, 2020
View changes
packages/router/src/route_reuse_strategy.ts Show resolved Hide resolved
packages/router/src/route_reuse_strategy.ts Outdated Show resolved Hide resolved
@abarghoud abarghoud force-pushed the export-DefaultRouteReuseStrategy branch from f342b71 to d8890d4 Compare Aug 12, 2020
@mary-poppins
Copy link

@mary-poppins mary-poppins commented Aug 12, 2020

You can preview d8890d4 at https://pr31575-d8890d4.ngbuilds.io/.

@abarghoud
fix(router): export DefaultRouteReuseStrategy to Router public_api
7698d26 
export DefaultRouteStrategy class that was used internally and exposed, and add documentation for each one of methods
@abarghoud abarghoud force-pushed the export-DefaultRouteReuseStrategy branch from d8890d4 to 7698d26 Compare Aug 12, 2020
@mary-poppins
Copy link

@mary-poppins mary-poppins commented Aug 12, 2020

You can preview 7698d26 at https://pr31575-7698d26.ngbuilds.io/.

@atscott
Copy link
Contributor

@atscott atscott commented Aug 13, 2020

@abarghoud Could you address the final minor comment from Alex? This should be good to go after that; all the approvals are there.

@abarghoud
Copy link
Contributor Author

@abarghoud abarghoud commented Aug 13, 2020

@abarghoud Could you address the final minor comment from Alex? This should be good to go after that; all the approvals are there.

@atscott I am not sure of which comment you are talking as for this one #31575 (comment) I've removed that paragraph and for #31575 (comment) I did the correction

@atscott
Copy link
Contributor

@atscott atscott commented Aug 13, 2020

@abarghoud My apologies! I was looking at the outdated comment #31575 (comment) that wasn't marked as resolved. I think everything looks good now.

@atscott atscott removed the request for review from pkozlowski-opensource Aug 13, 2020
@abarghoud abarghoud deleted the export-DefaultRouteReuseStrategy branch Aug 17, 2020
profanis added a commit to profanis/angular that referenced this issue Sep 5, 2020
@abarghoud @profanis
fix(router): export DefaultRouteReuseStrategy to Router public_api (a…
7c6b2d5 
…ngular#31575)

export DefaultRouteStrategy class that was used internally and exposed, and add documentation for each one of methods

PR Close angular#31575
@angular-automatic-lock-bot
Copy link

@angular-automatic-lock-bot angular-automatic-lock-bot bot commented Sep 17, 2020

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

@atscott atscott

@jelbourn jelbourn

@alxhub alxhub

@AndrewKushnir AndrewKushnir

Assignees
No one assigned
Labels
action: merge aio: preview cla: yes comp: router target: major
Projects
None yet
Milestone
needsTriage
Linked issues

Successfully merging this pull request may close these issues.

Export DefaultRouteReuseStrategy from router
9 participants
@abarghoud @santoshyadavdev @googlebot @atscott @mary-poppins @jelbourn @alxhub @AndrewKushnir @matsko