Code review comment for lp:~jtv/launchpad/pre-832647

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

> 189 + def _composeActiveSourcePubsCondition(self, distroseries, pocket):
>
> It seems slightly odd to have this take distroseries and pocket, but grab
> archive from self. This is a common problem in Soyuz, since archives didn't
> exist when most of it was written. Something to think about, maybe.

It did not escape notice. But the API reflects the current API of the dominator itself, and will have to rearranged in conjunction with it.

> 234 + And(join_spr_spn(), join_spph_spr(), spph_location_clauses),
>
> This would possibly be clearer if the order of arguments was reversed. The
> (SPR, SPN) -> (SPPH, SPR) -> SPPH chaining takes a while to pick up here.

True, it helps. I switched the first two around, but kept join conditions before selection conditions.

> 319 + def listCreaitonDates(self, spphs):
>
> Unused and misspelled :(

Removed.

« Back to merge proposal