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
merged 9 commits into from Aug 3, 2017

Conversation

Projects
None yet
4 participants
@wi-ski
Contributor

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

This comment has been minimized.

Contributor

ichernev commented Jan 29, 2017

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

This comment has been minimized.

Contributor

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 from Developmentsetter garbage fixusing isNaN to Setter garbage fix using isNaN Feb 14, 2017

@ichernev

This comment has been minimized.

Contributor

ichernev commented Mar 11, 2017

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

@westy92

This comment has been minimized.

westy92 commented Mar 13, 2017

The check claims he has:
image

@wi-ski

This comment has been minimized.

Contributor

wi-ski commented Mar 14, 2017

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

@maggiepint maggiepint merged commit 6ae49d6 into moment:develop Aug 3, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@ichernev ichernev changed the title from Setter garbage fix using isNaN to [feature] Ignore NaN values in setters Aug 6, 2017

ichernev added a commit that referenced this pull request Aug 6, 2017

Merge pull request #4106 from ichernev:drop-is-numeric
[fixup] Drop isNumeric utility fn, fixes #3735
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment