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

[feature] Ignore NaN values in setters #3735

Merged

Conversation

wi-ski
Copy link
Contributor

@wi-ski wi-ski commented Jan 29, 2017

I have issues with this approach - would like to discuss. Primary issue is that I could not get the previous tests passing with this implementation. Meaning, this implementation is intentionally gimped.

@ichernev
Copy link
Contributor

Unless I' missing something you can remove isNumber impl and tests, as it is not used. Looks good, we should make the moment invalid on invalid set for v3.

@wi-ski
Copy link
Contributor Author

wi-ski commented Jan 29, 2017

@ichernev - Will remove and update PR. Will also rebase.

@ichernev ichernev added this to the 2.18.0 milestone Feb 4, 2017
@wi-ski wi-ski changed the title Developmentsetter garbage fixusing isNaN Setter garbage fix using isNaN Feb 14, 2017
@ichernev
Copy link
Contributor

@wi-ski you haven't signed the CLA, so can't really merge this yet.

@westy92
Copy link

westy92 commented Mar 13, 2017

The check claims he has:
image

@wi-ski
Copy link
Contributor Author

wi-ski commented Mar 14, 2017

@westy92 @ichernev - I apologize. I signed the agreement but never acknowledged having done it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants