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

refactor(KitchenSink): remove thisArg from distinctUntilChanged signature #993

Closed
wants to merge 1 commit into from

Conversation

staltz
Copy link
Member

@staltz staltz commented Dec 8, 2015

Remove an obsolete thisArg from the signature of distinctUntilChanged in KitchenSink file.

For issue #878.

…ture

Remove an obsolete thisArg from the signature of distinctUntilChanged in KitchenSink file.

For issue ReactiveX#878.
@kwonoj
Copy link
Member

kwonoj commented Dec 8, 2015

think commit message need to updated to distinctUntilKeyChanged

@kwonoj
Copy link
Member

kwonoj commented Dec 8, 2015

  • requires to update implementation of distinctUntilKeyChanged,
if (compare) {
      return compare.call(thisArg, x[key], y[key]);
    }
...

@staltz
Copy link
Member Author

staltz commented Dec 8, 2015

Ouch, I didn't notice it was distinctUntilKeyChanged, not distinctUntilChanged.

@kwonoj
Copy link
Member

kwonoj commented Dec 8, 2015

@Blesh Actually this need to be merged to close out #878 completely - I'll check in once PR's updated to include changes.

@benlesh
Copy link
Member

benlesh commented Dec 9, 2015

Merged with 58c4ea9. Thanks @staltz

@benlesh benlesh closed this Dec 9, 2015
@kwonoj
Copy link
Member

kwonoj commented Dec 9, 2015

@Blesh , is this PR updated? I thought @staltz might need update to correctly reflect distinctUntilKeyChanged.

@benlesh
Copy link
Member

benlesh commented Dec 9, 2015

I thought it was.

@benlesh
Copy link
Member

benlesh commented Dec 9, 2015

I reviewed it and I thought it was supposed to be distinctUntilChanged. Confused by the title, I guess.

@benlesh
Copy link
Member

benlesh commented Dec 9, 2015

Looks correct to me: staltz@7cc3cbd#diff-e14b61d383aecb7508d52902e077098dL11

Is it just the PR title that was wrong?

@kwonoj
Copy link
Member

kwonoj commented Dec 9, 2015

I thought it was supposed to be distinctUntilChanged

: @staltz did also :) distinctUntilChanged is already in place, and this PR is about distinctUntilKeyChanged. and for distinctUntilKeyChanged, implementation still have pieces of using thisArg so PR might need to contains those changes to completely get away from thisArg for this operator.

@kwonoj
Copy link
Member

kwonoj commented Dec 9, 2015

This comment #993 (comment) is piece pointing implementation of distinctUntilKeyChanged .

@kwonoj
Copy link
Member

kwonoj commented Dec 9, 2015

Anyway, since this PR's already landed separate PR could be created to close distinctUntilKeyChanged.

@staltz staltz deleted the fix-kitchen-sink-thisArg branch December 9, 2015 08:09
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants