[webkit-reviews] review denied: [Bug 88709] nrwt outputs empty files for wdiff output if wdiff is not installed : [Attachment 148500] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 88709: nrwt outputs empty files for wdiff output if wdiff is not installed
https://bugs.webkit.org/show_bug.cgi?id=88709

Attachment 148500: Patch
https://bugs.webkit.org/attachment.cgi?id=148500&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=148500&action=review


Is there a test for this?

Do we need to update the code that generates results.html to make sure it
doesn't include links to the non-existent diffs? We should have a test for
that.

> Tools/Scripts/webkitpy/layout_tests/port/base.py:239
> +    def _log_wdiff_missing(self):
> +	   _log.warning("wdiff is not installed; please install it to generate
word-by-word diffs.")
> +	   _log.warning('')

I would change this method to just return the string to output.  Then you don't
have to duplicate the calls to _log.warning and the calls to _log.warning are
next to the 'if logging' check.