-
Notifications
You must be signed in to change notification settings - Fork 818
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
refactor: tslint cleanup #1704
refactor: tslint cleanup #1704
Conversation
a7fd976
to
3af5f65
Compare
Codecov Report
@@ Coverage Diff @@
## master #1704 +/- ##
==========================================
- Coverage 47.23% 47.17% -0.06%
==========================================
Files 41 41
Lines 1806 1808 +2
Branches 166 166
==========================================
Hits 853 853
- Misses 950 952 +2
Partials 3 3
Continue to review full report at Codecov.
|
3af5f65
to
3e83069
Compare
I'll look it it. It's just a lot of changes, so may take me some time |
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.
Hey, terence, thanks for your submission. I made some changes. One of the changes will may require more tslint updates, if you have the time. Thanks.
|
||
export function createMVCEventObservable<T>(array: MVCArray<T>): Observable<MVCEvent<T>>{ | ||
const eventNames = ['insert_at', 'remove_at', 'set_at']; | ||
return fromEventPattern( | ||
(handler: Function) => eventNames.map(evName => array.addListener(evName, | ||
(index: number, previous?: T) => handler.apply(array, [ {'newArr': array.getArray(), evName, index, previous} as MVCEvent<T>]))), | ||
(handler: Function, evListeners: MapsEventListener[]) => evListeners.forEach(evListener => evListener.remove())); | ||
(_handler: Function, evListeners: MapsEventListener[]) => evListeners.forEach(evListener => evListener.remove())); |
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.
why is this renamed? i kept it the same, so it matches its sibling parameter
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.
TSlint was complaining that it was unused and only didn't complain when it was prefixed with a "_"
2c65e15
to
e1516a9
Compare
@doom777 I made some updates to the PR based on your requests. |
Configure tslint properly + add some more checks and run
yarn lint --fix
to fix most and manually fix the rest.For old branches/PRs run
git checkout 3af5f65 package.json tslint.json && yarn && yarn lint --fix
and the branch should be easier to merge w/ these changes.