Bug 173093 – Gardening, fix more specific linter error

• Component: New Bugs
• Status: RESOLVED FIXED

[Charlie Turner]

Gardening, fix more specific linter error

[Charlie Turner]

Created attachment 312290 [details]
Patch

[Charlie Turner]

Created attachment 312291 [details]
Patch

[Carlos Alberto Lopez Perez]

Comment on attachment 312291 [details]
Patch

Note that to fix those warnings, usually what should be done is not to remove the expectation for Debug builds, but make the generic one be specific to Release:

--- a/LayoutTests/platform/gtk/TestExpectations
+++ b/LayoutTests/platform/gtk/TestExpectations
@@ -994,7 +994,7 @@ webkit.org/b/141700 fast/dom/Element/getClientRects.html [ Failure ]
 webkit.org/b/141700 fast/dom/Range/getClientRects.html [ Failure ]
 
 webkit.org/b/141705 media/video-fullscreeen-only-controls.html [ Failure ]
-webkit.org/b/141709 http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients.html [ Failure ]
+webkit.org/b/141709 [ Release ] http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients.html [ Failure ]
 webkit.org/b/141709 http/tests/security/cross-frame-access-callback-explicit-domain-ALLOW.html [ Failure ]
 
 webkit.org/b/141831 svg/animations/animate-text-nested-transforms.html [ Failure ]



However, in this case you comment that it not longer crashes on Debug builds, so then is right to just remove the expectation for Debug.

[Carlos Alberto Lopez Perez]

Comment on attachment 312291 [details]
Patch

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

> LayoutTests/ChangeLog:7
> +
> +        Reviewed by NOBODY (OOPS!).
> +

Also, for gardening commits its not needed review.
Next time you upload a gardening patch, change that line by something that contains the unreviewed word like:

"Unreviewed GTK gardening."

And ask any committer to cq+ the patch.

[Carlos Alberto Lopez Perez]

Comment on attachment 312291 [details]
Patch

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

> LayoutTests/ChangeLog:4
> +        Test gardening
> +        https://bugs.webkit.org/show_bug.cgi?id=173093

Also next time include the [GTK] tag at the beginning of the the title if the gardening is specific to the GTK port.

[Carlos Alberto Lopez Perez]

Comment on attachment 312291 [details]
Patch

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

> LayoutTests/ChangeLog:9
> +        Commit 40ba0c40a added a crash expectation for the following test, but did so in a manner
> +        that failed linter checks:

And don't reference commits by git hash, but by SVN number. WebKit uses SVN.

[Carlos Alberto Lopez Perez]

Comment on attachment 312291 [details]
Patch

Too much minor things.. I guess...

Can you please upload the patch again correcting those minor issues I commented above?

Tahnkls

[Charlie Turner]

Created attachment 312298 [details]
Patch

[Carlos Alberto Lopez Perez]

Comment on attachment 312298 [details]
Patch

Thanks

[WebKit Commit Bot]

Comment on attachment 312298 [details]
Patch

Clearing flags on attachment: 312298

Committed r217929: <http://trac.webkit.org/changeset/217929>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.