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
Patch (8.10 KB, patch)
2011-11-16 14:24 PST, Peter Rybin
no flags
Patch (7.82 KB, patch)
2011-11-17 06:06 PST, Peter Rybin
no flags
Peter Rybin
Comment 1 2011-11-02 09:43:48 PDT
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
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
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.