<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Add PerfAutoRun to automate browser based performance benchmarks(e.g. Speedometer and JetStream)"
href="https://bugs.webkit.org/show_bug.cgi?id=144038#c6">Comment # 6</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Add PerfAutoRun to automate browser based performance benchmarks(e.g. Speedometer and JetStream)"
href="https://bugs.webkit.org/show_bug.cgi?id=144038">bug 144038</a>
from <span class="vcard"><a class="email" href="mailto:cdumez@apple.com" title="Chris Dumez <cdumez@apple.com>"> <span class="fn">Chris Dumez</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=251306&action=diff" name="attach_251306" title="Patch">attachment 251306</a> <a href="attachment.cgi?id=251306&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=251306&action=review">https://bugs.webkit.org/attachment.cgi?id=251306&action=review</a>
<span class="quote">> Tools/Scripts/PerfAutoRun/README.md:110
> + * use ```shell git diff --relative HEAD >> your.patch``` to create your patch</span >
what is "shell" ?
Shouldn't it be > your.patch ? so that it actually creates the file?
<span class="quote">> Tools/Scripts/PerfAutoRun/README.md:112
> +* Create a plan for the benchmark(refer to **"How to create a plan"** for more details)</span >
Missing space before parenthesis.
<span class="quote">> Tools/Scripts/PerfAutoRun/README.md:113
> +* Do following instruction **ONLY IF NEEDED**, in most case, you do not have to.</span >
Follow these instructions...
in most cases,
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:2
> +# coding : utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:6
> +import json</span >
Please sort the imports alphabetically.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:16
> +from Utils import timeout</span >
Can be merged with the previous from Utils import.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:77
> + # wait for http server to launch and platform handler to clean</span >
Missing capital letter.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:78
> + # the environment</span >
Missing '.' at the end.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:94
> + logger.error('No result. Something goes wrong')</span >
"went wrong" ?
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/GenericHandleFactory.py:2
> +# coding : utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/GenericHandleFactory.py:6
> +import logging</span >
alphabetical sorting.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPHandle.py:2
> +# coding : utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPHandle.py:9
> + def serve(self, webRoot, serveCnt):</span >
What does "serveCnt" stand for? Please avoid abbreviations.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPHandleFactory.py:2
> +# coding : utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPServer/NaiveHTTPServer.py:2
> +# coding:utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPServer/NaiveHTTPServer.py:7
> +import posixpath</span >
alphabetical sorting.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPServer/NaiveHTTPServer.py:69
> + self.running = True</span >
isRunning, we like using prefixes for boolean variables.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPServer/TwistedHTTPServer.py:2
> +# coding: utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPServer/TwistedHTTPServer.py:28
> + backdoor = BackdoorPage()</span >
Not sure backdoor is a great name. It sounds scary :)
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:2
> +# coding : utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:24
> + # FIXME: this may not be reliable</span >
Missing '.' at the end.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:25
> + logger.info('finding the ip address of current machine')</span >
"Finding", "IP"
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:32
> + logger.error('cannot get the ip address of current machine')</span >
Missing capital letter.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:50
> + logger.info('start to fetching the port number of the http server')</span >
Missing capital letter.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:88
> + 'Http Server is serving at port: %d',</span >
HTTP
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/OSXChromeHandle.py:2
> +# coding : utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/OSXSafariHandle.py:35
> + args = [browserBuildPath + '/Safari.app/Contents/MacOS/Safari']</span >
os.path.join()
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/OSXSafariHandle.py:50
> + safaris = NSRunningApplication.runningApplicationsWithBundleIdentifier_(</span >
"safariInstances"? or simply "instances"?
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/PlatformHandle.py:2
> +# coding : utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/PlatformHandleFactory.py:2
> +# coding : utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/ResultWrapper/ResultWrapperFactory.py:2
> +# coding : utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/Utils.py:2
> +# coding: utf-8</span >
Same comment as before.
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/Utils.py:25
> +def getPathFromProjRoot(relativePathToProjRoot):</span >
Avoid abbreviations, Proj -> Project</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>