[webkit-reviews] review denied: [Bug 48326] [NRWT] Don't use image hash when it's no need in single test mode : [Attachment 71885] proposed patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied Gabor Rapcsanyi
<rgabor at inf.u-szeged.hu>'s request for review:
Bug 48326: [NRWT] Don't use image hash when it's no need in single test mode
https://bugs.webkit.org/show_bug.cgi?id=48326

Attachment 71885: proposed patch
https://bugs.webkit.org/attachment.cgi?id=71885&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=71885&action=review

Can you move this into a function call so the logic can be shared with the
logic on line 521?

>
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_threa
d.py:202
> +	   image_hash = test_info.image_hash()
> +	   if (image_hash and
> +	       (self._test_args.new_baseline or self._test_args.reset_results
or
> +	       not self._options.pixel_tests)):
> +	       image_hash = ""

How about:
if self._test_args.new_baseline or self._test_args.reset_results or not
self._options.pixel_tests:
    image_hash = None
else:
    image_hash = test_info.image_hash()

I find that logic moe clear and readable. Also, I prefer using None to the
empty string. That's more consistent with python style.