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

Router resolve #524

Closed
wants to merge 11 commits into from
Closed

Router resolve #524

wants to merge 11 commits into from

Conversation

rupeshtiwari
Copy link
Contributor

Worked on creating ROUTER_RESOLVE_END action for the issue #358

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 92.593% when pulling 33ab9d1 on roopkt:router-resolve into ab7de5c on ngrx:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 92.593% when pulling 3a5fa0c on roopkt:router-resolve into ab7de5c on ngrx:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 92.547% when pulling da648ea on roopkt:router-resolve into ab7de5c on ngrx:master.

@rupeshtiwari
Copy link
Contributor Author

Finished creating ROUTER_RESOLVE_END action dispatching. Fixes #358

@@ -1,4 +1,4 @@
import { ActionReducer } from '@ngrx/store';
import { ActionReducer, Action } from '@ngrx/store';
Copy link

Choose a reason for hiding this comment

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

Is this an unused import?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes and I am doing another checkin to remove those unused namespaces.
Below are required only. Basically this file does not need to be changed.
import { InjectionToken } from '@angular/core';
import { ActionReducer } from '@ngrx/store';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please undo any changes to this file. This is my mistake.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 92.547% when pulling c728222 on roopkt:router-resolve into ab7de5c on ngrx:master.

@rupeshtiwari
Copy link
Contributor Author

@royling @brandonroberts Hi I resolved the merge issues and also removed the redundant namespace all checked in.

@rupeshtiwari
Copy link
Contributor Author

Hi @brandonroberts is there any plan to review my pull request ?

@rupeshtiwari
Copy link
Contributor Author

Hi @brandonroberts
Any idea when this PR will be reviewed ?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 92.863% when pulling d60b45d on roopkt:router-resolve into ab5ad56 on ngrx:master.

@brandonroberts
Copy link
Member

Left a comment here about this feature: #358 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants