RESOLVED FIXED 90525
[EFL] Simplify runTest function by defining default arguments.
https://bugs.webkit.org/show_bug.cgi?id=90525
Summary [EFL] Simplify runTest function by defining default arguments.
Krzysztof Czech
Reported 2012-07-04 00:58:32 PDT
Default arguments will simplify macro RUN_TEST. Function will only have one mandatory argument, callback test function.
Attachments
[EFL] Modified runTest function by added two default arguments. Callback set to "load,finished" and data argument set to 0. (3.19 KB, patch)
2012-07-04 01:04 PDT, Krzysztof Czech
no flags
Modified runTest function, added default arguments. Updating unit tests (3.25 KB, patch)
2012-07-04 07:43 PDT, Krzysztof Czech
cshu: review+
[EFL] Simplify runTest function. (3.18 KB, patch)
2012-07-09 03:29 PDT, Krzysztof Czech
no flags
Krzysztof Czech
Comment 1 2012-07-04 01:04:11 PDT
Created attachment 150734 [details] [EFL] Modified runTest function by added two default arguments. Callback set to "load,finished" and data argument set to 0.
Gyuyoung Kim
Comment 2 2012-07-04 01:58:53 PDT
From the viewpoint of reducing parameter, looks fine. BTW, generally, we don't write bug title to description field.
Gyuyoung Kim
Comment 3 2012-07-04 01:59:09 PDT
Comment on attachment 150734 [details] [EFL] Modified runTest function by added two default arguments. Callback set to "load,finished" and data argument set to 0. View in context: https://bugs.webkit.org/attachment.cgi?id=150734&action=review > Source/WebKit/efl/tests/test_ewk_view.cpp:30 > +static void ewkViewEditableGetCb(void* eventInfo, Evas_Object* o, void* data) I think it is good to mention this change to ChangeLog as well.
Krzysztof Czech
Comment 4 2012-07-04 07:43:14 PDT
Created attachment 150793 [details] Modified runTest function, added default arguments. Updating unit tests
Gyuyoung Kim
Comment 5 2012-07-04 21:45:53 PDT
Comment on attachment 150793 [details] Modified runTest function, added default arguments. Updating unit tests Looks fine.
Grzegorz Czajkowski
Comment 6 2012-07-04 23:19:17 PDT
LGTM.
Thiago Marcos P. Santos
Comment 7 2012-07-05 04:21:41 PDT
LGTM - 1 nit. The only thing I don't like is the title of this bug and the ChangeLog. I would use something like: [EFL] Simplify runTest by defining default arguments
Chang Shu
Comment 8 2012-07-06 10:55:19 PDT
Comment on attachment 150793 [details] Modified runTest function, added default arguments. Updating unit tests View in context: https://bugs.webkit.org/attachment.cgi?id=150793&action=review > Source/WebKit/efl/ChangeLog:3 > + [EFL] Modified runTest function by added two default arguments. Callback set to "load,finished" and data argument set to 0. I prefer the title of the bug to describe what's broken or what's missing instead of what's done in the patch.
Krzysztof Czech
Comment 9 2012-07-09 03:29:11 PDT
Created attachment 151223 [details] [EFL] Simplify runTest function.
WebKit Review Bot
Comment 10 2012-07-09 04:36:52 PDT
Comment on attachment 151223 [details] [EFL] Simplify runTest function. Clearing flags on attachment: 151223 Committed r122102: <http://trac.webkit.org/changeset/122102>
WebKit Review Bot
Comment 11 2012-07-09 04:36:58 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.