[PATCH 2 of 3] patchbomb: support --outgoing and revsets

[Patrick Mézard]

Le 25/06/12 03:22, Matt Harbison a écrit :
> Patrick Mezard <patrick <at> mezard.eu> writes:
> 
>>
>> # HG changeset patch
>> # User Patrick Mezard <patrick <at> mezard.eu>
>> # Date 1340552367 -7200
>> # Branch stable
>> # Node ID f50a558c29dbd087d4943a73b4a5f9dd86984470
>> # Parent  01df43616027a869ed4e9e39f8b75cc7392d6e79
>> patchbomb: support --outgoing and revsets
>>
>> With --outgoing, input revisions were passed to getoutgoing() before
>> being resolved.
>>
>> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
>> --- a/hgext/patchbomb.py
>> +++ b/hgext/patchbomb.py
>> @@ -274,7 +274,7 @@
>>          dest, branches = hg.parseurl(dest)
>>          revs, checkout = hg.addbranchrevs(repo, repo, branches, revs)
>>          if revs:
>> -            revs = [repo.lookup(rev) for rev in revs]
>> +            revs = [repo.lookup(r) for r in scmutil.revrange(repo, revs)]
> 
> This is probably well beyond the scope of what you are trying to do, but why not
> make addbranchrevs return revs as a revset, and then the 'if revs:' part can go
> away?  It would also let several more commands accept revsets in the process.

Maybe.

Fixing patchbomb was already two levels away from the actual work I intended to do initially. As you can see in the followup patch, I decided that patchbomb should not even know about discovery and especially discovery.addbranchrevs(). Using revsets may not be the right answer but I am confident ditching discovery was going in the right direction.

I think there are too many calls to addbranchrevs(). The function is not really documented and its behaviour is not obvious and its input parameters are tied to parseurl(). Basically, there are repeated variations of:

parseurl()
getpeer()
addbranchrevs()
findcommon/incoming/outgoing/whatever()

Is there a way to reduce all this apparent code duplication?

--
Patrick Mézard