| Summary: | [EFL] Cleanup the build from unused parameters in WebKit-efl | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jeongeun Kim <je_julie.kim> | ||||||
| Component: | WebKit EFL | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bunhere, cdumez, commit-queue, gyuyoung.kim, gyuyoung.kim, lucas.de.marchi, sergio | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Other | ||||||||
| OS: | Linux | ||||||||
| Attachments: |
|
||||||||
|
Description
Jeongeun Kim
2014-04-15 06:02:18 PDT
Created attachment 229367 [details]
Patch
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. 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, (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. Created attachment 229438 [details]
Patch
(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, LGTM. Comment on attachment 229438 [details] Patch Clearing flags on attachment: 229438 Committed r167346: <http://trac.webkit.org/changeset/167346> All reviewed patches have been landed. Closing bug. |