[webkit-reviews] review denied: [Bug 62114] "WebCore/css/makeprop.pl" and "WebCore/css/makevalues.pl" should take ENABLE_ flags into account : [Attachment 96402] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jun 16 10:38:29 PDT 2011
Eric Seidel <eric at webkit.org> has denied Chiculita Alexandru
<achicu at adobe.com>'s request for review:
Bug 62114: "WebCore/css/makeprop.pl" and "WebCore/css/makevalues.pl" should
take ENABLE_ flags into account
https://bugs.webkit.org/show_bug.cgi?id=62114
Attachment 96402: patch
https://bugs.webkit.org/attachment.cgi?id=96402&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=96402&action=review
> Source/WebCore/WebCore.gyp/scripts/action_csspropertynames.py:86
> + # The defines come in as one flat string. Split it up into distinct
arguments.
> + if '--defines' in options:
> + definesIndex = options.index('--defines')
> + if definesIndex + 1 < len(options):
> + splitOptions = shlex.split(options[definesIndex + 1])
> + if splitOptions:
> + options[definesIndex + 1] = ' '.join(splitOptions)
I would have written this as a helper function.
> Source/WebCore/WebCore.gyp/scripts/action_csspropertynames.py:-129
> - line = line.rstrip()
> - if line.startswith('#'):
> - line = ''
> - if line == '':
> - continue
> - if line in lineDict:
> - raise KeyError, 'Duplicate value %s' % line
> - lineDict[line] = True
Why is it OK to remove this?
> Source/WebCore/WebCore.gyp/scripts/action_cssvaluekeywords.py:90
> + # The defines come in as one flat string. Split it up into distinct
arguments.
> + if '--defines' in options:
> + definesIndex = options.index('--defines')
> + if definesIndex + 1 < len(options):
> + splitOptions = shlex.split(options[definesIndex + 1])
> + if splitOptions:
> + options[definesIndex + 1] = ' '.join(splitOptions)
Now for sure thsi should be a helper. Even if it remains copy/pasted at least
some day we could chose to share it if they both use the same named function.
> Source/WebCore/WebCore.gyp/scripts/action_cssvaluekeywords.py:-135
> - line = line.rstrip()
> - if line.startswith('#'):
> - line = ''
> - if line == '':
> - continue
> line = line.lower()
> - if line in lineDict:
> - raise KeyError, 'Duplicate value %s' % line
> - lineDict[line] = True
Same question as before.
More information about the webkit-reviews
mailing list