Github user itkach commented on the pull request:
https://github.com/apache/thrift/pull/476#issuecomment-101486364
> Tests for the browser side would be awesome
Existing browser tests pass when executed locally (make check with Java
server is successful and grunt test fails only two tests involving the very
long string because my node, or rather iojs, server pukes for some reason -
doesn't appear to be related to my changes), in addition to unit tests running
under node. If I understand correctly browser tests run in PhantomJS, which is
basically some version of WebKit, which is ECMA5 js environment - not that the
new code assumes ECMA5, but if concern is about js versions compatibility then
"browser" tests don't really give a lot of extra assurances compared to tests
under node. So I'm not quite sure what additional tests are you looking for.
Can you elaborate?
> and, as I said before, putting the copy functions in the libs instead of
copying them on every service would be nice.
When you say `in the libs` do you actually mean two places -
`nodejs/lib/thrift/` and `js/src/thrift.js`? Just to clarify - you would like
the new list/map copy functions to be added in (copied into) these two places
instead of a single place in `t_js_generator.cc`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
