[webkit-reviews] review requested: [Bug 42588] Web Inspector: CodeGeneratorInspector should be able to generate Backend part of Inspector interface. : [Attachment 62298] [patch] second iteration
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jul 22 08:12:19 PDT 2010
Ilya Tikhonovsky <loislo at chromium.org> has asked for review:
Bug 42588: Web Inspector: CodeGeneratorInspector should be able to generate
Backend part of Inspector interface.
https://bugs.webkit.org/show_bug.cgi?id=42588
Attachment 62298: [patch] second iteration
https://bugs.webkit.org/attachment.cgi?id=62298&action=review
------- Additional Comments from Ilya Tikhonovsky <loislo at chromium.org>
(From update of attachment 62278 [details])
WebCore/GNUmakefile.am:553
+ DerivedSources/WebCore/InspectorBackendDispatcher.cpp \
mind alphabetic order
fixed.
WebCore/WebCore.gyp/WebCore.gyp:481
+ '../inspector/CodeGeneratorInspector.pm',
Could you add a comment for this?
done.
WebCore/WebCore.gypi:
+ 'webcore_inspector_idl_files': [
is it referenced from other places?
no other references.
WebCore/inspector/CodeGeneratorInspector.pm:168
+ push(@backendHead, " ${backendClassName}* create(InspectorBackend*
backend) { return new ${backendClassName}(backend); }");
either remote create or make the constructor private
removed.
WebCore/inspector/CodeGeneratorInspector.pm:241
+ push(@function, "void
${backendClassName}::${functionName}(PassRefPtr<InspectorArray>" . (
scalar(@argsFiltered) ? " args)" : ")"));
Please add comment about "unused argument" warning/
done.
WebCore/inspector/CodeGeneratorInspector.pm:248
+ $type = "double";
can you take it from the type description?
fixed.
WebCore/inspector/CodeGeneratorInspector.pm:271
+ my @mapEntries = map("dispatchMap.add(String(\"$_\"),
&${backendClassName}::$_);", @methods);
Is explicit check really needed here?
removed.
WebCore/inspector/CodeGeneratorInspector.pm:271
+ my @mapEntries = map("dispatchMap.add(String(\"$_\"),
&${backendClassName}::$_);", @methods);
I think calling .set( would be more appropriate here.
all these entries are assigning at the first run.
WebCore/inspector/front-end/ElementsPanel.js:225
+ InspectorBackend.pushNodeByPathToFrontend(callId,
this._selectedPathOnReset.join(","));
We should eventually pass an array instead of string here.
In generally it is possible but other platforms are not migrated yet.
> All methods but constructor and dispatch should be private.
done
More information about the webkit-reviews
mailing list