FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(observable-array): reduce no longer ignores zero as initial value by surdu · Pull Request #6402 · NativeScript/NativeScript · GitHub

/ NativeScript Public

fix(observable-array): reduce no longer ignores zero as initial value - #6402

Merged
vakrilov merged 5 commits into
NativeScript:masterfrom
surdu:fix-reduce
Oct 19, 2018
ConversationCommits (5)ChecksFiles changed
Merged

fix(observable-array): reduce no longer ignores zero as initial value#6402
vakrilov merged 5 commits into
NativeScript:masterfrom
surdu:fix-reduce

Conversation

surdu commented Oct 12, 2018

Copy link
Copy Markdown
Contributor

PR Checklist

What is the current behavior?

When using passing an initial value of zero to .reduce() from ObservableArray, the initial value is ignored

What is the new behavior?

Initial value is used

Fixes #6385

ghost added the ♥ community PR label Oct 12, 2018
NickIliev mentioned this pull request Oct 12, 2018
Closed

vakrilov commented Oct 12, 2018

Copy link
Copy Markdown
Contributor

Hey @surdu
The code looks right. Do you think you can also take a look at the reduceRight method. It seems that it's pretty identical and probably requires the same fix.

surdu commented Oct 12, 2018

Copy link
Copy Markdown
Contributor Author

@vakrilov In the case of reduceRight, the initialValue argument is passed directly to the JS method, and not interrogated beforehand.

That being said, now I'm wondering if reduceRight will not fail also if the initialValue is omitted (which I'm pretty sure it does). I'll check it out later today 😉

surdu commented Oct 12, 2018

Copy link
Copy Markdown
Contributor Author

@vakrilov Now I realize that what I was wondering is exactly what you said 😆. Sorry, I was tired.

I've just pushed the tests and the fix for reduceRight as well 😉

ghost assigned vakrilov Oct 17, 2018
ghost added in progress and removed ♥ community PR labels Oct 17, 2018

vakrilov commented Oct 17, 2018

Copy link
Copy Markdown
Contributor

Hey @surdu - thanks again for the PR! LGTM!
We are currently in a code-freeze phase for the NativeScript 5.0 release. This means, it might take more time to merge PRs into master.

We will notify you if the is some action needed form your side.
Thanks for understanding and being awesome part of the NS community!

vakrilov commented Oct 19, 2018

Copy link
Copy Markdown
Contributor

test

vakrilov merged commit c0438df into NativeScript:master Oct 19, 2018
ghost removed the in progress label Oct 19, 2018
surdu deleted the fix-reduce branch October 19, 2018 14:23

lock Bot commented Oct 22, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

lock Bot locked and limited conversation to collaborators Oct 22, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

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

Assignees

vakrilov

Labels

cla: yes ♥ community PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ObservableArray.reduce from tns-core-modules doesn't handle initial value correctly

4 participants

Back | FazBrowse Home | New Git URL