Code review comment for lp:~jelmer/launchpad/sync-tbz2

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Hi Guilherme,

Thanks for the review.

On Fri, 2010-02-05 at 17:48 +0000, Guilherme Salgado wrote:
> On Thu, 2010-02-04 at 16:24 +0000, Jelmer Vernooij wrote:
> > === modified file 'lib/canonical/launchpad/scripts/logger.py'
> > --- lib/canonical/launchpad/scripts/logger.py 2009-10-16 18:07:01 +0000
> > +++ lib/canonical/launchpad/scripts/logger.py 2010-02-04 16:24:23 +0000
> [...]
> > @@ -847,19 +854,14 @@
> > Return the fetched filename if it was present in Librarian or None
> > if it wasn't.
> > """
> > - # XXX cprov 2007-01-10 bug=78683: Looking for files within ubuntu
> > - # only. It doesn't affect the usual sync-source procedure. However
> > - # it needs to be revisited for derivation, we probably need
> > - # to pass the target distribution in order to make proper lookups.
> > - ubuntu = getUtility(IDistributionSet)['ubuntu']
> Does this mean the bug above will be fixed too?
Partially - bug 78683 is about sync-source only working with Ubuntu, and
this change gets us further in that direction but there are still some
places where we hardcode 'ubuntu' in scripts/ftp-master/sync-source.py.

I've addressed the style issue you've raised and I'll ec2 land after I
finish testing on dogfood.

Cheers,

Jelmer

« Back to merge proposal