Summary: | [GTK] Remove legacy hack in CodeGeneratorGObject.pm | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Diego Pino <dpino> | ||||||||||
Component: | Bindings | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Enhancement | CC: | buildbot, cdumez, cgarcia, commit-queue, rniwa, xan.lopez | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Diego Pino
2013-06-12 07:58:12 PDT
Created attachment 204441 [details]
Patch
Created attachment 205012 [details]
Patch
Comment on attachment 205012 [details] Patch Rejecting attachment 205012 [details] from commit-queue. dpino@igalia.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights. Comment on attachment 205012 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=205012&action=review > Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:548 > + } elsif ($attribute->signature->extendedAttributes->{"ImplementedBy"}) { > + my $implementedBy = $attribute->signature->extendedAttributes->{"ImplementedBy"}; > + $implIncludes{"${implementedBy}.h"} = 1; I wonder if we can get rid of this here, since this is already done a few lines above in this function. > Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:551 > + push(@txtGetProps, " g_value_set_$gtype(value, ${convertFunction}${getterFunctionName}(" . join(", ", @getterArguments) . ")${postConvertFunction});\n"); > + } else { > + push(@txtGetProps, " g_value_set_$gtype(value, ${convertFunction}${getterFunctionName}(" . join(", ", @getterArguments) . ")${postConvertFunction});\n"); I wonder why these lines are duplicated, I know it's not introduced by you, but I think this can be simplified. If we can get rid of the implemented by check this could just be } else { push(@txtGetProps, " g_value_set_$gtype(value, ${convertFunction}${getterFunctionName}(" . join(", ", @getterArguments) . ")${postConvertFunction});\n"); } Comment on attachment 205012 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=205012&action=review >> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:551 >> + push(@txtGetProps, " g_value_set_$gtype(value, ${convertFunction}${getterFunctionName}(" . join(", ", @getterArguments) . ")${postConvertFunction});\n"); > > I wonder why these lines are duplicated, I know it's not introduced by you, but I think this can be simplified. If we can get rid of the implemented by check this could just be > > } else { > push(@txtGetProps, " g_value_set_$gtype(value, ${convertFunction}${getterFunctionName}(" . join(", ", @getterArguments) . ")${postConvertFunction});\n"); > } Correct. I agree with these suggestions. I will cook a new patch with this changes, and check the script generates the same code. Created attachment 208011 [details]
Patch
Comment on attachment 208011 [details] Patch Attachment 208011 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1330178 New failing tests: svg/batik/filters/feTile.svg Created attachment 208019 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.4
Comment on attachment 208011 [details] Patch Clearing flags on attachment: 208011 Committed r153697: <http://trac.webkit.org/changeset/153697> All reviewed patches have been landed. Closing bug. |