Hi Alex, On Tue, Aug 18, 2009 at 9:50 AM, Alex Ghitza<[email protected]> wrote: > > Hello sage-devel, > > I feel a bit guilty asking this after having assigned reviews to > people (the response has been great btw, thanks!), but I realise that > my Sage development education has a significant flaw: I don't know the > proper way to review an spkg. > > There is good documentation in the developer guide for (a) creating an > spkg and (b) reviewing patches in trac. However, I couldn't find > anything about reviewing spkg's, and some things are rather different > from just patches. For instance, does one have to build the spkg on > all platforms supported by Sage? Should one check that spkg-install > and SPKG.txt conform to the recommendations for making an spkg? Is > the procedure for experimental/optional spkg's different than the one > for standard spkg's? > > What I am looking for is a step-by-step description that would guide > me through that process of reviewing, so that I don't give a positive > review to something that will create a Sage botnet :) Does such a > document exist? If not, can the accumulated wisdom of sage-devel put > one together?
Here are what I have been doing so far when reviewing spkg's. * Extract the spkg and use Mercurial to see if all changes have been checked in with a proper username and a sensible commit message. Doing something like "hg status" should not result in any output to the command line. Where an spkg has patches in the patch directory, the command "hg status" would say that the patches in that directory are not checked in, and you can see a question mark in front of the path of the patches. In some cases where the person who updated the spkg is not familiar with Mercurial or does not know how to check in changes, I would check in changes in the person's name and inform the person about this. * Read through the file SPKG.txt of the spkg to make sure there is a relevant update message. I usually spell-check that file for spkg's that I review. * Use "hg log" to determine the latest update patches and read through the patches to make sure that they make sense technically. For some changes, I think the person making the changes should document their reasons for doing so. It could be a few lines of comment in the relevant file or something like that. The idea is that, changes should be documented in the spkg. And on the relevant trac ticket, one needs to provide a high-level description of the proposed changes. * Build the spkg on any platform that I have access to. If the spkg claims to add support for Solaris, I would build it on Solaris under t2.math. I would also build it on sage.math and my local machines. * What follows is installing the spkg using "sage -f /path/to/package.spkg" and run all doctests under SAGE_ROOT/devel/sage. It's not enough to only run doctests that the spkg touches. This usually entails running all the doctests on sage.math. My local machines are too slow to finish all the doctests under one hour, and I want to use them as well while running the doctests. Sometimes I let the doctests run overnight on the computer in my office and check on them the next day. These would run on an Ubuntu host and a Gentoo guest running within VirtualBox, so running doctests overnight is a good option in my case. -- Regards Minh Van Nguyen --~--~---------~--~----~------------~-------~--~----~ To post to this group, send an email to [email protected] To unsubscribe from this group, send an email to [email protected] For more options, visit this group at http://groups.google.com/group/sage-devel URLs: http://www.sagemath.org -~----------~----~----~----~------~----~------~--~---
