Bug 131677 - [EFL] Cleanup the build from unused parameters in WebKit-efl
Summary: [EFL] Cleanup the build from unused parameters in WebKit-efl
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-15 06:02 PDT by Jeongeun Kim
Modified: 2014-04-16 05:26 PDT (History)
7 users (show)

See Also:


Attachments
Patch (4.00 KB, patch)
2014-04-15 06:08 PDT, Jeongeun Kim
no flags Details | Formatted Diff | Diff
Patch (1.57 KB, patch)
2014-04-16 04:42 PDT, Jeongeun Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeongeun Kim 2014-04-15 06:02:18 PDT
There are some unused params on WebKit-efl.
Comment 1 Jeongeun Kim 2014-04-15 06:08:21 PDT
Created attachment 229367 [details]
Patch
Comment 2 Gyuyoung Kim 2014-04-15 08:42:37 PDT
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.
Comment 3 Jeongeun Kim 2014-04-15 19:11:26 PDT
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,
Comment 4 Gyuyoung Kim 2014-04-15 20:18:57 PDT
(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.
Comment 5 Jeongeun Kim 2014-04-16 04:42:48 PDT
Created attachment 229438 [details]
Patch
Comment 6 Jeongeun Kim 2014-04-16 04:54:19 PDT
(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,
Comment 7 Gyuyoung Kim 2014-04-16 04:56:47 PDT
LGTM.
Comment 8 WebKit Commit Bot 2014-04-16 05:26:06 PDT
Comment on attachment 229438 [details]
Patch

Clearing flags on attachment: 229438

Committed r167346: <http://trac.webkit.org/changeset/167346>
Comment 9 WebKit Commit Bot 2014-04-16 05:26:12 PDT
All reviewed patches have been landed.  Closing bug.