[webkit-reviews] review denied: [Bug 68321] [EFL] Use C++ type cast instead of C style type cast : [Attachment 108441] Patch

[bugzilla-daemon at webkit.org]

Lucas De Marchi <demarchi at webkit.org> has denied Gyuyoung Kim
<gyuyoung.kim at samsung.com>'s request for review:
Bug 68321: [EFL] Use C++ type cast instead of C style type cast
https://bugs.webkit.org/show_bug.cgi?id=68321

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

------- Additional Comments from Lucas De Marchi <demarchi at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108441&action=review


> Source/WebKit/efl/ChangeLog:60
> +	   * ewk/ewk_auth_soup.cpp:
> +	   (_Ewk_Auth_Data::ewk_auth_soup_credentials_set):
> +	   (_Ewk_Auth_Data::session_authenticate):
> +	   * ewk/ewk_contextmenu.cpp:
> +	   (ewk_context_menu_item_new):
> +	   * ewk/ewk_frame.cpp:
> +	   (_ewk_frame_smart_add):
> +	   (ewk_frame_children_iterator_new):
> +	   (ewk_frame_hit_test_new):
> +	   * ewk/ewk_history.cpp:
> +	   (_ewk_history_item_new):
> +	   (ewk_history_item_title_get):
> +	   (ewk_history_item_title_alternate_get):
> +	   (ewk_history_item_uri_get):
> +	   (ewk_history_item_uri_original_get):
> +	   (ewk_history_new):
> +	   * ewk/ewk_view.cpp:
> +	   (_ewk_view_repaints_resize):
> +	   (_ewk_view_scrolls_resize):
> +	   (_ewk_view_on_focus_in):
> +	   (_ewk_view_on_focus_out):
> +	   (_ewk_view_on_mouse_wheel):
> +	   (_ewk_view_on_mouse_down):
> +	   (_ewk_view_on_mouse_up):
> +	   (_ewk_view_on_mouse_move):
> +	   (_ewk_view_on_key_down):
> +	   (_ewk_view_on_key_up):
> +	   (_ewk_view_priv_new):
> +	   (_ewk_view_smart_add):
> +	   (_ewk_view_smart_zoom_set):
> +	   (_ewk_view_zoom_animator_cb):
> +	   (_ewk_view_viewport_attributes_compute):
> +	   (ewk_view_zoom_set):
> +	   (ewk_view_zoom_weak_set):
> +	   (ewk_view_zoom_animated_set):
> +	   (ewk_view_paint_context_new):
> +	   (ewk_view_popup_new):
> +	   (ewk_view_popup_destroy):
> +	   * ewk/ewk_view_single.cpp:
> +	   (_ewk_view_single_smart_add):
> +	   (_ewk_view_single_smart_resize):
> +	   (_ewk_view_single_scroll_process_single):
> +	   (_ewk_view_single_smart_repaints_process):
> +	   * ewk/ewk_view_tiled.cpp:
> +	   (_ewk_view_tiled_render_cb):
> +	   (_ewk_view_tiled_updates_process_pre):
> +	   (_ewk_view_tiled_contents_size_changed_cb):
> +	   (_ewk_view_tiled_smart_add):

As mentioned by Antonio Gomes in the other giant patch, please remove this from
ChangeLog since it's not adding any useful information.

> Source/WebKit/efl/ewk/ewk_view.cpp:1564
> -	   WRN("zoom level is < %f : %f",
(double)priv->settings.zoom_range.min_scale, (double)zoom);
> +	   WRN("zoom level is < %f : %f",
static_cast<double>(priv->settings.zoom_range.min_scale),
static_cast<double>(zoom));

I'm surprised to see we are casting a float to a double. This is a nice time to
remove that.

> Source/WebKit/efl/ewk/ewk_view.cpp:1568
> -	   WRN("zoom level is > %f : %f",
(double)priv->settings.zoom_range.max_scale, (double)zoom);
> +	   WRN("zoom level is > %f : %f",
static_cast<double>(priv->settings.zoom_range.max_scale),
static_cast<double>(zoom));

ditto.

> Source/WebKit/efl/ewk/ewk_view.cpp:1608
> -	   WRN("zoom level is < %f : %f",
(double)priv->settings.zoom_range.min_scale, (double)zoom);
> +	   WRN("zoom level is < %f : %f",
static_cast<double>(priv->settings.zoom_range.min_scale),
static_cast<double>(zoom));

ditto.

> Source/WebKit/efl/ewk/ewk_view.cpp:1612
> -	   WRN("zoom level is > %f : %f",
(double)priv->settings.zoom_range.max_scale, (double)zoom);
> +	   WRN("zoom level is > %f : %f",
static_cast<double>(priv->settings.zoom_range.max_scale),
static_cast<double>(zoom));

ditto.

> Source/WebKit/efl/ewk/ewk_view.cpp:1664
> -	   WRN("zoom level is < %f : %f",
(double)priv->settings.zoom_range.min_scale, (double)zoom);
> +	   WRN("zoom level is < %f : %f",
static_cast<double>(priv->settings.zoom_range.min_scale),
static_cast<double>(zoom));

ditto.

> Source/WebKit/efl/ewk/ewk_view.cpp:1668
> -	   WRN("zoom level is > %f : %f",
(double)priv->settings.zoom_range.max_scale, (double)zoom);
> +	   WRN("zoom level is > %f : %f",
static_cast<double>(priv->settings.zoom_range.max_scale),
static_cast<double>(zoom));

ditto.