search

strongloop / strong-remoting

Communicate between objects in servers, mobile apps, and other servers.
www.strongloop.com
Other
105 stars 93 forks source link

Introduce support for integer datatype #323

Closed gunjpan closed 8 years ago

gunjpan commented 8 years ago

all types of numbers, in Javascript, are of only one dataType: Numbers. This is a POC Patch to fix #317. Note: Breaks few test cases on local

Connect to #317

gunjpan commented 8 years ago

@bajtos : Here is the PoC for introducing integer data types. PTAL and let's discuss if the direction is correct. Please note, CI will fail as it had six failing test cases on local, which will be fixed if we're moving further this direction

gunjpan commented 8 years ago

/cc @superkhau @raymondfeng

bajtos commented 8 years ago

@gunjpan Looks mostly good 👍 Please add a unit-test for "array of integers", address my comments above and fix test failures. I think adding support for "integer" should not be a breaking change, i.e. all existing tests should keep passing.

bajtos commented 8 years ago

@gunjpan there is one more thing we will need to do as part of fixing the issue, and that's adding new tests to rest-coercion integration tests in test/rest-coercion. I slightly prefer to do it as part of this patch, to ensure we don't miss any edge case here, but I won't mind if you decide to do that in a follow-up pull request.

gunjpan commented 8 years ago

@bajtos : Previous comments addressed. PTAL at the latest commit. Thanks.

bajtos commented 8 years ago

Two more comments, you are almost there :)

bajtos commented 8 years ago

LGTM. Please squash the commits and wait for CI build to turn green before landing.

gunjpan commented 8 years ago

@slnode test please

gunjpan commented 8 years ago

@slnode test please

gunjpan commented 8 years ago

@bajtos: Landed. Do we need to backport it to 2.x?