<div dir="ltr">On 9 September 2013 16:46, Dylan Baker <span dir="ltr"><<a href="mailto:baker.dylan.c@gmail.com" target="_blank">baker.dylan.c@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="im">On Monday 09 September 2013 16:34:08 you wrote:<br>
> This series changes the way testlists are generated. This allows us to<br>
> generate a TestProfile object only once, at build time. This is<br>
> accomplished by using python's pickle module, which stores a python<br>
> object representation as a plain text file.<br>
><br>
> The first 9 patches touch all of the testfiles the majority of these<br>
> patches simply replace ``from <foo> import *'' with explicit imports.<br>
> This is the recomended way of doing things from the python devs. r600 is<br>
> removed, since it *is* quick-driver.tests, as is external-glparser.tests<br>
> since it is deprecated.<br>
><br>
> The last patch does all of the heavy lifting, moving all of the tests<br>
> files to a new folder, and adding the CMake magic to generate the<br>
> testslists<br>
><br>
> There are a couple of reasons for doing this:<br>
> 1) Since piglit-run is no longer responsible for producing<br>
>    the TestProfile objects, it is much simpler to change the format<br>
>    they are stored it. This is to our advantage as we try to create a<br>
>    python3 branch of piglit since ``execfile'' is deprecated in python3<br>
><br>
> 2) This takes a few seconds off the start of piglit-run, since all of<br>
>    the heavy liftiner of running the python *tests files are removed,<br>
>    instead being done durring build time<br>
<br>
</div>This is also available at my github:<br>
<a href="https://github.com/crymson/piglit.git" target="_blank">https://github.com/crymson/piglit.git</a> generate-tests<br></blockquote><div><br></div><div>I see one patch on the github branch that I don't see on the mailing list:<br>
<br>ae04a6b all.tests: Replace import * with only the necissary functions<br><br></div><div>In that patch, s/necissary/necessary/.<br><br></div><div>General comment on all the patches that replace "import *" with specific imports: rather than the variations on "this is bad" in the commit messages, I would just quote the relevant text from PEP 8, which says:<br>
<br>    "Wildcard imports (from <module> import *) should be avoided, as they make it unclear which names are present in the namespace, confusing both readers and many automated tools."<br><br></div><div>This will be helpful in case someone goes digging through git history later trying to understand why you did what you did.<br>
<br></div><div>With that change, all patches but the last are:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br></div><div><br></div><div>Regarding the last patch, it looks like the mailing list ate it because all the renamed files made it too big.  I'm going to re-generate the patch using "--find-renames" and re-send it; then I'll reply to it with my comments.<br>
</div></div></div></div>