Merge ~nacc/git-ubuntu:make-clone-verbose into git-ubuntu:master

I agree in principle, although it feels to me that we should stick to either "verbose" or "quiet" but not both (so I suggest "verbose" as we've used that elsewhere), and that they should be independent of must_exist. So no magic of the defaulting or setting of one based on the other - leave that to the caller.

That's my gut feel, without having looked or thought deeper.

On Tue, Aug 1, 2017 at 2:39 PM, Robie Basak <email address hidden> wrote:
>
>
> Diff comments:
>
>> diff --git a/gitubuntu/git_repository.py b/gitubuntu/git_repository.py
>> index ba0480b..fb7179b 100644
>> --- a/gitubuntu/git_repository.py
>> +++ b/gitubuntu/git_repository.py
>> @@ -265,12 +265,17 @@ class GitUbuntuRepository:
>> # XXX: want a remote alias of 'lpme' -> self.lp_user
>> # self.git_run(['config', 'url.%s.insteadof' % self.lp_user, 'lpme'])
>>
>> - def _fetch_remote(self, remote_name, must_exist):
>> + def fetch_remote(self, remote_name, must_exist, quiet=True):
>> try:
>> # Does not seem to be working with https
>> # self.raw_repo.remotes[remote_name].fetch()
>> logging.debug('Fetching remote %s' % remote_name)
>> - self.git_run(['fetch', remote_name], quiet=not must_exist)
>> + kwargs = {}
>> + kwargs['quiet'] = not must_exist
>
> This is the magic crossing of the two concepts that I think is a little surprising and should be left to the caller instead.

100% agree :) hence the glance, as you tend to be able to think a bit
faster about best design in Python.

I'll adjust and resubmit later this week.

Looks good! One minor style comment inline.

Thanks, I found some practical errors and forgot to update queue. Will send
a new MP tomorrow.

On Aug 2, 2017 16:49, "Robie Basak" <email address hidden> wrote:

> Review: Approve
>
> Looks good! One minor style comment inline.
>
> Diff comments:
>
> > diff --git a/gitubuntu/importer.py b/gitubuntu/importer.py
> > index 9f466c9..bbfc3b5 100644
> > --- a/gitubuntu/importer.py
> > +++ b/gitubuntu/importer.py
> > @@ -37,7 +37,7 @@ import tempfile
> > import time
> > from gitubuntu.cache import CACHE_PATH
> > from gitubuntu.dsc import GitUbuntuDsc
> > -from gitubuntu.git_repository import GitUbuntuRepository, orphan_tag,
> applied_tag, import_tag, upstream_tag
> > +from gitubuntu.git_repository import GitUbuntuRepository,
> GitUbuntuRepositoryFetchError, orphan_tag, applied_tag, import_tag,
> upstream_tag
>
> Going >79 here? If so, I think we can do:
>
> from gitubuntu.git_respository import (
> a,
> b,
> c,
> )
>
> I could be wrong about the exact syntax, but I believe something along
> these lines is possible.
>
> > from gitubuntu.run import decode_binary, run, runq
> > from gitubuntu.source_information import GitUbuntuSourceInformation,
> NoPublicationHistoryException, SourceExtractionException,
> launchpad_login_auth
> > from gitubuntu.version import VERSION
>
>
> --
> https://code.launchpad.net/~nacc/usd-importer/+git/usd-
> importer/+merge/328486
> You are the owner of ~nacc/usd-importer:make-clone-verbose.
>