Bug 136312

*** Bug 136311 has been marked as a duplicate of this bug. ***

Created attachment 244879 [details] Patch

This patch modifies the WEB_REPLAY inputs generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-input-generator-tests --reset-results`)

Attachment 244879 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-with-guard.json-TestReplayInputs.h:71: The parameter name "phase" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/replay/scripts/CodeGeneratorReplayInputs.py:795: whitespace before '}' [pep8/E202] [5] ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-encoding-helpers-with-guarded-values.json-TestReplayInputs.h:60: The parameter name "button" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-encoding-helpers.json-TestReplayInputs.h:81: The parameter name "button" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 4 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 244922 [details] Patch (w/ Windows fix)

Comment on attachment 244922 [details] Patch (w/ Windows fix) View in context: https://bugs.webkit.org/attachment.cgi?id=244922&action=review r=me > Source/WebCore/ChangeLog:11 > + * replay/WebInputs.json: Moved common types to JSINputs.json. Typo: "JSINputs.json" > Source/JavaScriptCore/replay/scripts/CodeGeneratorReplayInputs.py:787 > + if len(self.setting('exportMacro')) > 0: Would just "if len(...):" do? > Source/JavaScriptCore/replay/scripts/CodeGeneratorReplayInputs.py:981 > + if len(input_filepaths) == 0: This could also be "if !len(...):". Do what you think is clearer. > Source/WebCore/DerivedSources.make:1164 > INPUT_GENERATOR_SPECIFICATIONS = \ > $(WebCore)/replay/WebInputs.json \ > + $(WebReplayScripts)/JSInputs.json \ > # I wonder if we should split this up into two variables so one day someone doesn't try to sort these in a different order and inadvertently break things since the order here is important. Nothing to do here unless you want to.

Comment on attachment 244922 [details] Patch (w/ Windows fix) View in context: https://bugs.webkit.org/attachment.cgi?id=244922&action=review Will rebase and EWS one more time before landing. >> Source/JavaScriptCore/replay/scripts/CodeGeneratorReplayInputs.py:787 >> + if len(self.setting('exportMacro')) > 0: > > Would just "if len(...):" do? There are only a few places in WebKit python code that use such implicit conversions with len(), so I'm shy to add more. >> Source/WebCore/DerivedSources.make:1164 >> # > > I wonder if we should split this up into two variables so one day someone doesn't try to sort these in a different order and inadvertently break things since the order here is important. > > Nothing to do here unless you want to. The order is *not* important. Specifications are not type-checked (i.e., try to lookup all type references between files) until all input files are parsed. Maybe I should put that into the changelog.

Created attachment 244924 [details] Patch (for EWS)

Created attachment 244925 [details] Patch (for EWS)

Attachment 244925 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-with-guard.json-TestReplayInputs.h:71: The parameter name "phase" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/replay/scripts/CodeGeneratorReplayInputs.py:806: whitespace before '}' [pep8/E202] [5] ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-encoding-helpers-with-guarded-values.json-TestReplayInputs.h:60: The parameter name "button" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-encoding-helpers.json-TestReplayInputs.h:81: The parameter name "button" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 4 in 42 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 244927 [details] Patch (always Windows...)

Created attachment 244935 [details] Patch (for EWS, 5)

Created attachment 244969 [details] Patch (EWS)

Committed r178714: <http://trac.webkit.org/changeset/178714>

(In reply to comment #14) > Committed r178714: <http://trac.webkit.org/changeset/178714> Based on the reviewed patch with fix for windows (locally confirmed to fix known build issue). Landed manually since I was unable to convince EWS to apply changes to copy-files.cmd, which address the windows issue. I'll be watching in case Windows still has problems.