There is not formal size limit for any patch, some changes are
inherently big.
Experience shows that big patch tends to have more hidden bugs and
poorer code coverage. It is better to go forward by little step while
having huge test coverage at every step.
https://bugs.webkit.org/attachment.cgi?id=245350&action=review looks
hard to split.
Maybe do a first patch with an empty shell + the bindings. You can test
passing all kind of valid and invalid values to the API; test the
property of the object and every function/properties, test creating
thousands of ReadableStream objects, etc.
Then a follow up patch implementing the ReadableStream and have an other
set of test checking the behavior?
On 1/26/15 3:22 AM, youenn fablet wrote:
The latest patch at https://bugs.webkit.org/show_bug.cgi?id=138967
resolves the crash (some JSC::Strong<> were missing).
I fear that the patch may be a bit too big to get a thorough review though.
The patch could be split into meaningful but not testable sub-patches
(module stuff, JS integration stuff and then tests and more tests).
Would that make sense?
What is a reasonable patch size limit?
Any advice well appreciated.
Thanks,
Youenn
2015-01-21 19:40 GMT+01:00 Xabier RodrÃguez Calvar <calva...@igalia.com>:
Hi!
I am now implementing with Youenn the Streams API standard [1] in
WebKit. You can find the first patch at [2] (it's r? now). While we get
that patch reviewed and landed we are adding more tests and working out
the problems. One of them is one crash that I cannot hunt, with the
following backtrace:
http://fpaste.org/172619/60635142/
You can find the code under the lines to make it easier. What is going
on is:
1. There's a call to the ReadableStream object, delegated to the
JSReadableStreamSource as a result of the object creation.
2. There's a call to the JSReadableStream::read method, delegating
in the ReadableStream that ends up pulling again and that second
call crashes.
It is probably something stupid I am not taking into account, but I have
already been fighting this for a couple of days and cannot make it work
properly.
Any help? Thanks a lot in advance!
[1] https://streams.spec.whatwg.org/
[2] https://bugs.webkit.org/show_bug.cgi?id=138967
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
