On Wednesday 11 September 2013 10:36:41 Paul Berry wrote: > On 9 September 2013 16:46, Dylan Baker <[email protected]> wrote: > > On Monday 09 September 2013 16:34:08 you wrote: > > > This series changes the way testlists are generated. This allows us to > > > generate a TestProfile object only once, at build time. This is > > > accomplished by using python's pickle module, which stores a python > > > object representation as a plain text file. > > > > > > The first 9 patches touch all of the testfiles the majority of these > > > patches simply replace ``from <foo> import *'' with explicit imports. > > > This is the recomended way of doing things from the python devs. r600 is > > > removed, since it *is* quick-driver.tests, as is external-glparser.tests > > > since it is deprecated. > > > > > > The last patch does all of the heavy lifting, moving all of the tests > > > files to a new folder, and adding the CMake magic to generate the > > > testslists > > > > > > There are a couple of reasons for doing this: > > > 1) Since piglit-run is no longer responsible for producing > > > > > > the TestProfile objects, it is much simpler to change the format > > > they are stored it. This is to our advantage as we try to create a > > > python3 branch of piglit since ``execfile'' is deprecated in python3 > > > > > > 2) This takes a few seconds off the start of piglit-run, since all of > > > > > > the heavy liftiner of running the python *tests files are removed, > > > instead being done durring build time > > > > This is also available at my github: > > https://github.com/crymson/piglit.git generate-tests > > I see one patch on the github branch that I don't see on the mailing list: > > ae04a6b all.tests: Replace import * with only the necissary functions > > In that patch, s/necissary/necessary/. > > 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: > > "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." > > This will be helpful in case someone goes digging through git history later > trying to understand why you did what you did. > > With that change, all patches but the last are: > > Reviewed-by: Paul Berry <[email protected]> > > 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.
I have made the appropriate changes to patches 1-9 and will send them upstream. Thanks for the review.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
