[Piuparts-devel] short review on daves branches

[Dave Steele]

On Sat, May 26, 2012 at 9:56 AM, Andreas Beckmann <debian at abeckmann.de> wrote:
> * remotes/dave/purge_order
> That is wrong. We already do what you want to achieve (according to the
> changelog):
>  - purge the dependencies (deps_to_purge)
>  - purge the package under test afterwards (nondeps_to_purge)
>    feel free to rename the variables to something understandable :-)
>  (and yes, we want to keep the "insane" purge order:
>   dependencies first, package-under-test last)
>

OK. My main driver in pursuing this in the short term is to drive down
the dependency-failed counts in a defensible fashion. Either #672769
or your exclude list could accomplish that.

> * remotes/dave/report-performance
> * 5e62551a8ed replace O(n^2) search in remove_old_logs with a hash
> - nice catch, although I don't see this as a big time hog, e.g.

Granted ... but there is another, far worse, one

> - the big time eater is "Writing package templates"
>  that's writing 17000 files per section

That's where the far worse one is :-)

>
> *  remotes/dave/rrdeps
> I'm curious, cherry-picked and tested it  :)
...
> - should this be done for dependency-failed-testing,
> dependency-does-not-exist and dependency-cannot-be-tested, too?
> - the numbers should be separated by ', '
>

rrdeps was a proof-of-concept - more about infrastructure than
interface, though it sounds like the display is close to good enough.
I'll look at making the changes you describe (the
dependency-does-not-exist case is problematic).

> Oops: this happened:
>  File "/org/piuparts.debian.org/share/piuparts/piuparts-report", line
> 1126, in write_state_pages
>    "aside": aside,
> UnboundLocalError: local variable 'aside' referenced before assignment
>

Shouldn't be possible. 'aside' is defined in both paths of an if
statement above the error. Could this be a merge issue?

Mmm, unless there are no packages. I'll fix.