180303 2017-12-01 18:20:48 -0800 Add docstring in EarlyWarningSystemTask to explain return values 2017-12-04 16:06:40 -0800 1 1 1 Unclassified WebKit Tools / Tests Other Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 aakash_jain aakash_jain aakash_jain ap dbates dean_johnson ews-watchlist glenn jbedard lforschler webkit-bug-importer oldest_to_newest 1377207 0 aakash_jain 2017-12-01 18:20:48 -0800 EarlyWarningSystemTask run() method has return value which are slightly non-intuitive. False does not indicate that patch failed EWS, it indicates that EWS is not sure. Patch failure is indicated by an exception. We should document these values in order to avoid confusion. Having this comment could have prevented https://bugs.webkit.org/show_bug.cgi?id=180302 1377208 1 328202 aakash_jain 2017-12-01 18:23:59 -0800 Created attachment 328202 Proposed patch 1377234 2 328202 dbates 2017-12-01 21:52:46 -0800 Comment on attachment 328202 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=328202&action=review > Tools/Scripts/webkitpy/tool/bot/earlywarningsystemtask.py:61 > + """ Could named constants help make this clearer And let us simplify this comment to just explain that an exception indicates patch failure (e.g. RETRY = False, PASS => True)? If you have your heart set on this comment then “would be” => “will be”. Also does the formatting of this docstring match the formatting of other docstrings that explain return values and/or exceptions? 1377971 3 aakash_jain 2017-12-04 16:05:05 -0800 Committed r225500: <https://trac.webkit.org/changeset/225500> 1377975 4 webkit-bug-importer 2017-12-04 16:06:40 -0800 <rdar://problem/35842175> 328202 2017-12-01 18:23:59 -0800 2017-12-04 16:05:32 -0800 Proposed patch webkitpy_earlywarningsystemtask_docstring.patch text/plain 1369 aakash_jain SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDIyNTQ0MCkKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDEzIEBACisyMDE3LTEyLTAxICBBYWthc2ggSmFpbiAgPGFha2FzaF9qYWluQGFwcGxlLmNv bT4KKworICAgICAgICBBZGQgZG9jc3RyaW5nIGluIEVhcmx5V2FybmluZ1N5c3RlbVRhc2sgdG8g ZXhwbGFpbiByZXR1cm4gdmFsdWVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0xODAzMDMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvdG9vbC9ib3QvZWFybHl3YXJuaW5nc3lz dGVtdGFzay5weToKKyAgICAgICAgKEVhcmx5V2FybmluZ1N5c3RlbVRhc2sucnVuKTogQWRkZWQg ZG9jc3RyaW5nIHRvIGV4cGxhaW4gcmV0dXJuIHZhbHVlcy4KKwogMjAxNy0xMi0wMSAgRGV3ZWkg Wmh1ICA8ZGV3ZWlfemh1QGFwcGxlLmNvbT4KIAogICAgICAgICBIYXJkY29kZWQgcHl0aG9uIHBh dGggaXMgbm90IGNvbXBhdGlibGUgd2l0aCB2aXJ0dWFsIGVudmlyb25tZW50LgpJbmRleDogVG9v bHMvU2NyaXB0cy93ZWJraXRweS90b29sL2JvdC9lYXJseXdhcm5pbmdzeXN0ZW10YXNrLnB5Cj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFRvb2xzL1NjcmlwdHMvd2Via2l0cHkvdG9vbC9ib3QvZWFybHl3YXJuaW5n c3lzdGVtdGFzay5weQkocmV2aXNpb24gMjI1NDQwKQorKysgVG9vbHMvU2NyaXB0cy93ZWJraXRw eS90b29sL2JvdC9lYXJseXdhcm5pbmdzeXN0ZW10YXNrLnB5CSh3b3JraW5nIGNvcHkpCkBAIC01 NCw2ICs1NCwxMSBAQCBjbGFzcyBFYXJseVdhcm5pbmdTeXN0ZW1UYXNrKFBhdGNoQW5hbHlzCiAg ICAgICAgIHJldHVybiBUcnVlCiAKICAgICBkZWYgcnVuKHNlbGYpOgorICAgICAgICAiIiIKKyAg ICAgICAgcmV0dXJucyBUcnVlOiBwYXRjaCBwYXNzZXMgRVdTLgorICAgICAgICByZXR1cm5zIEZh bHNlOiBwYXRjaCBzdGF0dXMgY2FuIG5vdCBiZSBkZXRlcm1pbmVkIGZvciBzdXJlLiBQYXRjaCB3 b3VsZCBiZSByZS10cmllZCBieSBhbm90aGVyIGJvdC4KKyAgICAgICAgZXhjZXB0aW9uOiBwYXRj aCBmYWlscyBFV1MuCisgICAgICAgICIiIgogICAgICAgICBpZiBub3Qgc2VsZi5fY2xlYW4oKToK ICAgICAgICAgICAgIHJldHVybiBGYWxzZQogICAgICAgICBpZiBub3Qgc2VsZi5fdXBkYXRlKCk6 Cg==