GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix(router): make locationSyncBootstrapListener public due to change … by jasonaden · Pull Request #22896 · angular/angular

Conversation

@jasonaden

@jasonaden jasonaden commented Mar 20, 2018

Copy link
Copy Markdown
Contributor

…in output after TS 2.7 update in #22669

See build diff on 2.7 here:

angular/router-builds@8788b39#diff-068e657ab436c99f423c5ad8d4187c1b

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@googlebot googlebot added the cla: yes label Mar 20, 2018
@jasonaden
jasonaden requested a review from IgorMinar March 20, 2018 20:58
IgorMinar
IgorMinar suggested changes Mar 20, 2018
View reviewed changes

@IgorMinar IgorMinar left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

can't you type the useFactory prop as (ngUpgrade: UpgradeModule) => () => void by casting the initializer value?

we shouldn't export random private stuff just to make the compiler happy.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mary-poppins

mary-poppins commented Mar 20, 2018

Copy link
Copy Markdown

You can preview de94b19 at https://pr22896-de94b19.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasonaden
fix(router): make locationSyncBootstrapListener public due to change …
37a5d18 
…in output after TS 2.7 update in angular#22669
@jasonaden
jasonaden force-pushed the fix_router_upgrade branch from de94b19 to 37a5d18 Compare March 20, 2018 21:41
@jasonaden

jasonaden commented Mar 20, 2018

Copy link
Copy Markdown
Contributor Author

@IgorMinar Sounds good. Updated.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
IgorMinar
IgorMinar approved these changes Mar 20, 2018
View reviewed changes
@IgorMinar IgorMinar added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release labels Mar 20, 2018
@ngbot

ngbot Bot commented Mar 20, 2018

Copy link
Copy Markdown

I see that you just added the PR action: merge label, but the following checks are still failing:
    failure status "ci/circleci: lint" is failing
    pending status "ci/circleci: build" is pending
    pending status "google3" 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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@IgorMinar

IgorMinar commented Mar 20, 2018

Copy link
Copy Markdown
Contributor

@jasonaden please reformat
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mary-poppins

mary-poppins commented Mar 20, 2018

Copy link
Copy Markdown

You can preview 37a5d18 at https://pr22896-37a5d18.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasonaden
fixup! fix(router): make locationSyncBootstrapListener public due to …
86e68b1 
…change in output after TS 2.7 update in angular#22669
@mary-poppins

mary-poppins commented Mar 20, 2018

Copy link
Copy Markdown

You can preview 86e68b1 at https://pr22896-86e68b1.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@IgorMinar IgorMinar closed this in 623d769 Mar 21, 2018
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 13, 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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker cla: yes target: major This PR is targeted for the next major release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@jasonaden @mary-poppins @IgorMinar @googlebot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.