Bug 131677 – [EFL] Cleanup the build from unused parameters in WebKit-efl

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Jeongeun Kim]

There are some unused params on WebKit-efl.

[Jeongeun Kim]

Created attachment 229367 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 229367 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229367&action=review

> Source/WebKit/efl/ewk/ewk_network.cpp:-56
> -void ewk_network_tls_certificate_check_set(Eina_Bool checkCertificates)

This isn't to remove only parameter name. This is to remove parameter itself. So, you have to say why you should change function argument.

[Jeongeun Kim]

Gyuyoung, Thank you for your feedback.
(In reply to comment #2)
> > Source/WebKit/efl/ewk/ewk_network.cpp:-56
> > -void ewk_network_tls_certificate_check_set(Eina_Bool checkCertificates)
> 
> This isn't to remove only parameter name. This is to remove parameter itself. So, you have to say why you should change function argument.

I thought rule for removing parameter name is for cases that there is already predefined prototype.
For instance, parent class has virtual member API with several params but child class which has real implementation doesn't need some of params. Or, some cases depending on Feature Enabled.

In this case,
ewk_network_tls_certificate_check_set is  API based on C (I mean there is no dependency) and I thought that it's not necessary any more.

How do you think?
Thanks,

[Gyuyoung Kim]

(In reply to comment #3)
> Gyuyoung, Thank you for your feedback.
> (In reply to comment #2)
> > > Source/WebKit/efl/ewk/ewk_network.cpp:-56
> > > -void ewk_network_tls_certificate_check_set(Eina_Bool checkCertificates)
> > 
> > This isn't to remove only parameter name. This is to remove parameter itself. So, you have to say why you should change function argument.
> 
> I thought rule for removing parameter name is for cases that there is already predefined prototype.
> For instance, parent class has virtual member API with several params but child class which has real implementation doesn't need some of params. Or, some cases depending on Feature Enabled.
> 
> In this case,
> ewk_network_tls_certificate_check_set is  API based on C (I mean there is no dependency) and I thought that it's not necessary any more.
> 
> How do you think?
> Thanks,

ASSERT_UNUSED() is matched a rule to remove unused parameter. However, in ewk_network_tls_certificate_check_set() case, you are changing function behavior. So, ewk_network_tls_certificate_check_set() needs to be handled in new bug.

[Jeongeun Kim]

Created attachment 229438 [details]
Patch

[Jeongeun Kim]

(In reply to comment #4)
> ASSERT_UNUSED() is matched a rule to remove unused parameter. However, in ewk_network_tls_certificate_check_set() case, you are changing function behavior. So, ewk_network_tls_certificate_check_set() needs to be handled in new bug.

Gyuyoung, thank you for your comment.
I fixed unused param warning only on ewk_view_scroll with this issue.
Please look into it.
Thanks,

[Gyuyoung Kim]

LGTM.

[WebKit Commit Bot]

Comment on attachment 229438 [details]
Patch

Clearing flags on attachment: 229438

Committed r167346: <http://trac.webkit.org/changeset/167346>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.