Made suggested changes. Re-ran unit tests. Please re-review.
http://codereview.chromium.org/7889046/diff/1/include/v8.h File include/v8.h (right): http://codereview.chromium.org/7889046/diff/1/include/v8.h#newcode2455 include/v8.h:2455: ExternalAsciiStringResourceImpl() : data_(0), length_(0) {}; On 2011/09/19 07:58:57, fschneider wrote:
No ; at the end?
Fixed (et seq.). Thanks. Still a bit rusty on C++ :(. http://codereview.chromium.org/7889046/diff/1/include/v8.h#newcode2460 include/v8.h:2460: private: On 2011/09/19 07:58:57, fschneider wrote:
Newline before private:
Fixed. http://codereview.chromium.org/7889046/diff/1/include/v8.h#newcode2476 include/v8.h:2476: int source_length = -1); On 2011/09/19 07:58:57, fschneider wrote:
I wish there was a way to reduce the number of optional paramters in a
row. E.g.
does the source parameter have to be optional?
Agreed. I'd appreciate your opinion on altering vs. extending the existing v8 API. I am taking the conservative route. http://codereview.chromium.org/7889046/diff/1/test/cctest/test-api.cc File test/cctest/test-api.cc (right): http://codereview.chromium.org/7889046/diff/1/test/cctest/test-api.cc#newcode4298 test/cctest/test-api.cc:4298: for (int source_len = kEmbeddedExtensionSourceValidLen; On 2011/09/19 07:58:57, fschneider wrote:
Why does this loop go from kEmbeddedExtensionSourceValidLen to kEmbeddedExtensionSourceValidLen? Shouldn't it start from 0?
Thank you for catching this. I have changed the range to [-1, +1] (relative to the constant). I was doing some debugging and neglected to return to the original range before uploading. Now fixed. http://codereview.chromium.org/7889046/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
