[webkit-reviews] review granted: [Bug 173559] lint-test-expectations should be run during style checking : [Attachment 314760] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jul 18 15:23:28 PDT 2017


David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has granted Jonathan Bedard
<jbedard at apple.com>'s request for review:
Bug 173559: lint-test-expectations should be run during style checking
https://bugs.webkit.org/show_bug.cgi?id=173559

Attachment 314760: Patch

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




--- Comment #10 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 314760
  --> https://bugs.webkit.org/attachment.cgi?id=314760
Patch

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

r=me, although this patch is complex enough it wouldn't hurt to get a second
set of eyes.

> Tools/ChangeLog:11
> +	   Running the test expectation linter requires reading both files and
lines not in the
> +	   patch because, for example, deletion of a test can cause a lint
failure even though
> +	   not test expectations where modified.

I feel there's more to be said here.  What is the implication of this
statement?  Maybe something like this?

This means that the linter may occasionally warn about lines that weren't
changed in the immediate patch, but which were triggered by those changes. 
Care is taken to reduce the number of warnings outside the changes made within
a patch, though.

> Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:401
> +	   self.related_files = {}  # Dictionary of files to lines in that
file. Storing associated warnings

Nit: Comment should end with period.


More information about the webkit-reviews mailing list