WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
71372
Web Inspector: clear fixme in generator script
https://bugs.webkit.org/show_bug.cgi?id=71372
Summary
Web Inspector: clear fixme in generator script
Peter Rybin
Reported
2011-11-02 09:39:19 PDT
Clean fixme in Source/WebCore/inspector/CodeGeneratorInspector.py
Attachments
Patch
(5.54 KB, patch)
2011-11-02 09:43 PDT
,
Peter Rybin
no flags
Details
Formatted Diff
Diff
Patch
(8.10 KB, patch)
2011-11-16 14:24 PST
,
Peter Rybin
no flags
Details
Formatted Diff
Diff
Patch
(7.82 KB, patch)
2011-11-17 06:06 PST
,
Peter Rybin
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Peter Rybin
Comment 1
2011-11-02 09:43:48 PDT
Created
attachment 113322
[details]
Patch
Ilya Tikhonovsky
Comment 2
2011-11-02 10:17:04 PDT
looks good to me
Yury Semikhatsky
Comment 3
2011-11-03 00:54:27 PDT
Comment on
attachment 113322
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=113322&action=review
> Source/WebCore/inspector/CodeGeneratorInspector.py:109 > + field_name_res = domain_name.lower() + "Agent"
This will generate m_domstorageAgent instead of m_domStorageAgent as per coding style guidelines.
Peter Rybin
Comment 4
2011-11-03 03:44:42 PDT
Do you mean we have standards for generated code that high? (In reply to
comment #3
)
> (From update of
attachment 113322
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=113322&action=review
> > > Source/WebCore/inspector/CodeGeneratorInspector.py:109 > > + field_name_res = domain_name.lower() + "Agent" > > This will generate m_domstorageAgent instead of m_domStorageAgent as per coding style guidelines.
Pavel Feldman
Comment 5
2011-11-05 09:10:24 PDT
Comment on
attachment 113322
[details]
Patch As per yury's comment.
Peter Rybin
Comment 6
2011-11-16 14:24:45 PST
Created
attachment 115448
[details]
Patch
Ilya Tikhonovsky
Comment 7
2011-11-16 21:57:28 PST
Comment on
attachment 115448
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=115448&action=review
could you please also fix the indentation for InspectorBackendDispatcher.cpp the strings: if (!protocolErrors->length()) m_anAgent->aCommand(blablabla) sendResponse(callId, result, blablabla static CallHandler handlers[] = { &InspectorBackendDispatcher::Page_enable, and extract String::format("Some arguments of method '%s' can't be processed", "blablabla") as an inline function.
> Source/WebCore/inspector/CodeGeneratorInspector.py:106 > + def lower_camel_case_to_upper(str): > + if len(str) > 0 and str[0].islower(): > + str = str[0].upper() + str[1:] > + return str
unused. remove it.
> Source/WebCore/inspector/CodeGeneratorInspector.py:126 > + def camel_case_to_capitilized_with_underscores(str):
capitalized. unused - remove it.
Peter Rybin
Comment 8
2011-11-17 06:04:56 PST
Ilya, thank you for review.
> could you please also fix the indentation for InspectorBackendDispatcher.cpp > the strings:
Done
> and extract String::format("Some arguments of method '%s' can't be processed", "blablabla") > as an inline function.
I don't think this is really related. Let's handle it separately. By the way, I don't like its string formatting before the error actually happend anyway. On each message you allocate string buffer, build a string and never use it (until an actual error).
> > Source/WebCore/inspector/CodeGeneratorInspector.py:106 > unused. remove it.
Done
> > Source/WebCore/inspector/CodeGeneratorInspector.py:126 > capitalized. unused - remove it.
Done
Peter Rybin
Comment 9
2011-11-17 06:06:59 PST
Created
attachment 115574
[details]
Patch
Ilya Tikhonovsky
Comment 10
2011-11-17 06:28:04 PST
Comment on
attachment 115574
[details]
Patch lgtm
Ilya Tikhonovsky
Comment 11
2011-11-17 21:48:10 PST
Comment on
attachment 115574
[details]
Patch Clearing flags on attachment: 115574 Committed
r100723
: <
http://trac.webkit.org/changeset/100723
>
Ilya Tikhonovsky
Comment 12
2011-11-17 21:48:18 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug