Bug 173093 - Gardening, fix more specific linter error
Summary: Gardening, fix more specific linter error
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-08 04:29 PDT by Charlie Turner
Modified: 2017-06-08 06:24 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.11 KB, patch)
2017-06-08 04:29 PDT, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch (2.11 KB, patch)
2017-06-08 04:34 PDT, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch (2.10 KB, patch)
2017-06-08 06:11 PDT, Charlie Turner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Charlie Turner 2017-06-08 04:29:07 PDT
Gardening, fix more specific linter error
Comment 1 Charlie Turner 2017-06-08 04:29:27 PDT
Created attachment 312290 [details]
Patch
Comment 2 Charlie Turner 2017-06-08 04:34:07 PDT
Created attachment 312291 [details]
Patch
Comment 3 Carlos Alberto Lopez Perez 2017-06-08 05:53:35 PDT
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.
Comment 4 Carlos Alberto Lopez Perez 2017-06-08 05:54:55 PDT
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.
Comment 5 Carlos Alberto Lopez Perez 2017-06-08 05:56:31 PDT
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.
Comment 6 Carlos Alberto Lopez Perez 2017-06-08 05:57:05 PDT
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.
Comment 7 Carlos Alberto Lopez Perez 2017-06-08 05:58:03 PDT
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
Comment 8 Charlie Turner 2017-06-08 06:11:19 PDT
Created attachment 312298 [details]
Patch
Comment 9 Carlos Alberto Lopez Perez 2017-06-08 06:18:30 PDT
Comment on attachment 312298 [details]
Patch

Thanks
Comment 10 WebKit Commit Bot 2017-06-08 06:24:29 PDT
Comment on attachment 312298 [details]
Patch

Clearing flags on attachment: 312298

Committed r217929: <http://trac.webkit.org/changeset/217929>
Comment 11 WebKit Commit Bot 2017-06-08 06:24:31 PDT
All reviewed patches have been landed.  Closing bug.