[webkit-reviews] review denied: [Bug 87965] Need customized toDOMStringList for IndexedDB JSC binding. : [Attachment 147069] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Charles Wei
<charles.wei at torchmobile.com.cn>'s request for review:
Bug 87965: Need customized toDOMStringList for IndexedDB JSC binding.
https://bugs.webkit.org/show_bug.cgi?id=87965

Attachment 147069: Patch
https://bugs.webkit.org/attachment.cgi?id=147069&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=147069&action=review


>>> Source/WebCore/UseJSC.cmake:60
>>> +	 bindings/js/JSDOMStringListCustom.cpp
>> 
>> This would not be the only place where you need to add the file name.
>> 
>> $ grep -r JSDOMStringMapCustom.cpp *
>> 
>> will tell you the list of build files you need to touch.
> 
> Well, there are so many portings and we don't have all the building system.
We can only guarantee that all the systems that connect to the build-bot works,
I don't want to blindly add this to other systems without verification. Any
porting that are not added to the build-bot need to add this to their system by
themselves.

What ports are you intending to support?

> We can only guarantee that all the systems that connect to the build-bot
works

To guarantee that, you need to at least add the file to project.pbxproj (mac),
GNUmakefile.list.am (Gtk), Target.pri (qt), WebCore.vcproj (win) and
CMakeLists.txt (Efl). The reason why build-bots are green without adding the
file is simply that IndexedDB is not yet enabled on none of the build-bots that
use JSC. This is the reason why I do not want to land patches before enabling
IndexedDB. I am afraid that a bunch of stuff would break when IndexedDB is
enabled.

> Any porting that are not added to the build-bot need to add this to their
system by themselves.

At least for those build-bots, (basically) it is our responsibility to make it
work.