Bug 71372 - Web Inspector: clear fixme in generator script
Summary: Web Inspector: clear fixme in generator script
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P4 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-02 09:39 PDT by Peter Rybin
Modified: 2011-11-17 21:48 PST (History)
11 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Rybin 2011-11-02 09:39:19 PDT
Clean fixme in Source/WebCore/inspector/CodeGeneratorInspector.py
Comment 1 Peter Rybin 2011-11-02 09:43:48 PDT
Created attachment 113322 [details]
Patch
Comment 2 Ilya Tikhonovsky 2011-11-02 10:17:04 PDT
looks good to me
Comment 3 Yury Semikhatsky 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.
Comment 4 Peter Rybin 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.
Comment 5 Pavel Feldman 2011-11-05 09:10:24 PDT
Comment on attachment 113322 [details]
Patch

As per yury's comment.
Comment 6 Peter Rybin 2011-11-16 14:24:45 PST
Created attachment 115448 [details]
Patch
Comment 7 Ilya Tikhonovsky 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.
Comment 8 Peter Rybin 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
Comment 9 Peter Rybin 2011-11-17 06:06:59 PST
Created attachment 115574 [details]
Patch
Comment 10 Ilya Tikhonovsky 2011-11-17 06:28:04 PST
Comment on attachment 115574 [details]
Patch

lgtm
Comment 11 Ilya Tikhonovsky 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>
Comment 12 Ilya Tikhonovsky 2011-11-17 21:48:18 PST
All reviewed patches have been landed.  Closing bug.