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
fix(xxx-intl): replace misused EventEmitter with Subject #6313
Conversation
According to https://angular.io/api/core/EventEmitter#description, EventEmitter is not for services. And Subject is preferrable.
d684875
to
6459ca5
Compare
6459ca5
to
5c41c3a
Compare
@@ -6,7 +6,8 @@ | |||
* found in the LICENSE file at https://angular.io/license | |||
*/ | |||
|
|||
import {Injectable, EventEmitter} from '@angular/core'; | |||
import {Injectable} from '@angular/core'; | |||
import { Subject } from 'rxjs/Subject'; |
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.
As a heads up, they'll want you to remove these spaces. Here and below.
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.
that's ok. however, it's better to apply a tool like prettier that format the code at pre-commit. That would be more efficient on code style management.
The tests should be fine, there is something wrong with travis. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
According to https://angular.io/api/core/EventEmitter#description, EventEmitter is not for services. And Subject is preferrable.