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

Array.prototype.filter.not.forcing.boolean #7779

Merged

Conversation

NimaZahedi
Copy link
Contributor

Fixes #
Changed the return value of Array.prototype.filter, use any instead of boolean!
based on this issue: #5850

@msftclas
Copy link

msftclas commented Apr 1, 2016

Hi @NimaZahedi, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@RyanCavanaugh
Copy link
Member

Missing baseline file... run these commands:

  • jake runtests
  • jake baseline-accept
  • git add -A .
  • git commit -m "Add baselines"
  • git push

@NimaZahedi
Copy link
Contributor Author

@RyanCavanaugh Ok, but isn't the same as #7776? 😕

@DanielRosenwasser
Copy link
Member

@NimaZahedi Ryan means that we shouldn't even have generated an error baseline given your change.

@yuit does the test harness use the LKG's libs?

@RyanCavanaugh
Copy link
Member

Ping @yuit - which files are the correct ones to edit?

@NimaZahedi
Copy link
Contributor Author

@RyanCavanaugh @DanielRosenwasser
I've tried these two set of commands:

  • First
    • Make my changes (writing test and fix issue)
    • jake (updating built/local/ files d.ts)
    • jake runtests
    • jake baseline-accept
    • jake runtests
  • Second
    • Writing test
    • jake runtests
    • jake baseline-accept
    • Fix issue
    • jake (updating built/local/ files d.ts)
    • jake runtests

@yuit
Copy link
Contributor

yuit commented Apr 7, 2016

@DanielRosenwasser the harness will pick up lib files built to built\local. @NimaZahedi, the file you edit is correct, the issue here is that there are multiple interface of Array-ish (ReadonlyArray, Array, Int8Array etc.); There are around 11 of them. You will want to update them all.

@NimaZahedi
Copy link
Contributor Author

@yuit I'll do, sure. thanks :)

@NimaZahedi
Copy link
Contributor Author

NimaZahedi commented Apr 15, 2016

@yuit Should I write test case for every single of those types? (I mean the interfaces whose filter method is changed)

@RyanCavanaugh
Copy link
Member

👍

No need to write a test for every overload

@mhegazy
Copy link
Contributor

mhegazy commented Apr 18, 2016

👍

@yuit
Copy link
Contributor

yuit commented Apr 18, 2016

@NimaZahedi look goods :) Thanks!

@yuit yuit merged commit 3e2fff2 into microsoft:master Apr 18, 2016
@ghost ghost mentioned this pull request Nov 15, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
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.

6 participants