[arch-releng] Iso tests

Dan McGee dpmcgee at gmail.com
Thu Mar 10 11:04:24 EST 2011


On Thu, Mar 10, 2011 at 3:43 AM, Tom Willemsen
<tom.willemsen at archlinux.us> wrote:
> Hello people of arch-releng,
>
> A while ago Dieter asked if someone could write him a (relatively)
> simple web-app to get a better view of the state of the auto-generated
> isos. I tried that and have now come up with something to look at.
> He asked me to send a mail to this list so we could have a look at it in
> public, so here it is. It's hosted at:
> http://www.gitorious.org/ryuslash/archweb
>
> I just branched the archlinux website, since Dieter said he might want
> it to be a part of it, and it's written in django anyway so it's fairly
> easy to rip it out and place it in a new project.
> My code is in the testsresults branch in the afore mentioned repo, to
> set it up just follow the instructions in the archweb README, I added
> some fixtures so that things like bootloaders don't have to be entered
> manually by everyone.
> The /isotests/ page shows the results that have been entered so far, I
> tried sticking closely to what Dieter requested. The /isotests/add/ page
> allows you to add a new result.
>
> I would have liked to put the app in a repo of its own, but I didn't
> find how I can make the templates load from the app/templates directory,
> so I moved on to this idea.
>
> I don't have a lot of experience with django, so I could be doing some
> things in a very silly way, I haven't cleaned up the code I've written
> at all (I should probably do that soon, I'll get on it tonight), but I
> hope that this is at least an ok starting point.
>
> I hope I exlpained everything enough, if there's anything I still need
> to do, fix or change just let me know and I'll get on it as soon as I
> can.

Nice! I'd be glad to have this hosted on the main site (I'm the
maintainer). Overall things look sound; sure there are some
style/convention/etc. differences but nothing hard to fix up.

* View page: right now it is making 98 queries (one for each of those
max values). Not that this page is probably going to see loads of
traffic but we can surely improve upon that.
* Add page: Using radio buttons for widgets here would probably be
smarter than the dropdowns, otherwise field entry is excruciatingly
painful. See the forms documentation for how to change the default
widget.
* models, Test.ms: totally un-descriptive field name, and you didn't
give it a nice field name. Can't this just be "modules"?
* models, Test: if you name things boot_type instead of boottype,
you'll get nicer default labels on your view and entry screens. Same
with the models themselves- Boottype -> BootType.
* models, Iso: Likely need more than a date field here. We've double
released, etc. It might just be worth having a CharField as no users
are going to be entering these anyway, and also having a boolean
"active" field or something to determine what actually shows up on the
entry and view pages.
* models, get_success_test/get_failed_test: these can be abstracted
into a common proxy model
(http://docs.djangoproject.com/en/1.2/topics/db/models/#proxy-models)
superclass, from which all these various ISO options inherit from.
* tests.py: yes, we should be writing tests, but at the least, get rid
of the default garbage in this file.
* admin.py: convention is not to use * imports; I know it works here
and makes things cleaner but I've eradicated the codebase of them
otherwise.
* models.py: "# Create your models here." -> not needed
* urls.py: You aren't using info_dict anymore; you have a blank second
pattern definition?, and I'd prefer you follow indentation patterns of
something like packages/urls.py.
* views.py, add: A lot of extra comments. "Create your views here.",
all the "form been submitted" trailing stuff. For the return, I've
switched to using mostly direct_to_template to avoid some of the
boilerplate- see the end of flag() in packages/views.py around line
373 for an example.
* views.py:, view: I see some things are _choices, some are _list- why
the disparity?
* isotest/templates: why is this in a different directory from the
standard layout?
* Finally, I see 4 + 1 templates, but only two views- are these other
ones old, unnecessary, not wired up yet?

Great work! I know I just said a lot but these are mostly minor
things, nothing to feel bad about.

-Dan


More information about the arch-releng mailing list