[webkit-reviews] review granted: [Bug 177730] [Settings] Move remaining simple settings to Settings.in : [Attachment 324134] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 18 10:58:53 PDT 2017


Daniel Bates <dbates at webkit.org> has granted Sam Weinig <sam at webkit.org>'s
request for review:
Bug 177730: [Settings] Move remaining simple settings to Settings.in
https://bugs.webkit.org/show_bug.cgi?id=177730

Attachment 324134: Patch

https://bugs.webkit.org/attachment.cgi?id=324134&action=review




--- Comment #11 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 324134
  --> https://bugs.webkit.org/attachment.cgi?id=324134
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324134&action=review

> Source/WebCore/ChangeLog:19
> +	       Migrage simple settings from SettingsBase to here.

No need to rage about it.

Migrage => Migrate

> Source/WebCore/Scripts/GenerateSettings/Settings.py:97
> +	   if self.getter:
> +	       return self.getter
>	   return self.name

return self.getter or self.name

> Source/WebCore/Scripts/GenerateSettings/Settings.py:131
> +    for line in open(input, 'r'):

This is leaking the file handle. It is good programming practice to close the
file once we no longer need it. Either we need a close() or, we can use the
"with" keyword since File objects implement the context manager interface:

with open(input, 'r') as file:
    for line in file:
	...

See <https://docs.python.org/2/tutorial/inputoutput.html> for more details.


More information about the webkit-reviews mailing list