[Webkit-unassigned] [Bug 218440] Use FileDownload to fetch built product from master
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Nov 9 18:21:51 PST 2020
https://bugs.webkit.org/show_bug.cgi?id=218440
Aakash Jain <aakash_jain at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #413160|review?, commit-queue? |review+, commit-queue-
Flags| |
--- Comment #5 from Aakash Jain <aakash_jain at apple.com> ---
Comment on attachment 413160
--> https://bugs.webkit.org/attachment.cgi?id=413160
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=413160&action=review
Patch looks good. various minor comments.
Also you can remove EWS_BUILD_URL variable, it's unused after this patch.
> Tools/CISupport/ews-build/steps.py:2443
> +class DownloadBuiltProductFromMaster(transfer.FileDownload):
Please add step name, e.g.:
name = 'download-built-product-from-master'
> Tools/CISupport/ews-build/steps.py:2458
> def getResultSummary(self):
Nit: missing empty line before method.
> Tools/CISupport/ews-build/steps_unittest.py:211
> +def downloadFileRecordingContents(recorder):
Maybe it's better to keep this method inside TestDownloadBuiltProductFromMaster class since it's used only there.
> Tools/CISupport/ews-build/steps_unittest.py:2601
> + self.setupStep(step)
Can write this as one line:
self.setupStep(DownloadBuiltProductFromMaster(mastersrc=__file__))
> Tools/CISupport/ews-build/steps_unittest.py:2621
> + buf = b''.join(buf)
is 'b' required in b''?
> Tools/CISupport/ews-build/steps_unittest.py:2622
> + self.assertEqual(len(buf), 1000)
Can store this 1000 in a variable and re-use that variable.
> Tools/CISupport/ews-build/steps_unittest.py:2623
> + with open(__file__, 'rb') as mf:
can improve the variable name for 'mf'
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20201110/eb3b957f/attachment-0001.htm>
More information about the webkit-unassigned
mailing list