Re: [StGIT PATCH 3/4] Convert git_id() to the new id format

From: Catalin Marinas <catalin.marinas@gmail.com>
Date: Jul 13, 2008, 11:20 UTC
Message-ID: <b0943d9e0807130420v37e36f30sef8b7745fa55b688@mail.gmail.com>
In-Reply-To: <20080622154854.GC4468@diana.vm.bytemark.co.uk>

2008/6/22 Karl Hasselström <kha@treskal.com>:

Show 6 quoted lines

> On 2008-06-19 22:42:22 +0100, Catalin Marinas wrote:
>
>> The patch rewrites git_id() to use the new id format and coverts the
>> commands using this function. The git_id() will be removed once all
>> the commands are converted to the new infrastructure where
>> git_commit() will be used instead.

[...[

Show 6 quoted lines

>>      if not rev:
>> +        # backwards compatibility
>>          return None
>
> Could you expand this comment a bit? It's not enough of a clue for me.
> :-/

I removed it, the diff command used to pass None as rev2 if the user only passed one boundary of the range. I fixed diff and removed the above.

Show 5 quoted lines

>> -def git_commit(name, repository, branch = None):
>> +def git_commit(name, repository, branch_name = None):
>
> Very nice parameter rename here, now that we have Branch objects (and
> use a crappy language with no type system).

It has a type system but no compile-time checking (I'm more in favour of static typing but no time to rewrite stgit :-)).

Show 9 quoted lines

>> -        (refpatchname, refbranchname, refpatchid) = parse_rev(patchname)
>> -        if refpatchname and not refpatchid and \
>> -               (not refpatchid or refpatchid == 'top'):
>> -            # FIXME: should also support picking //top.old
>> +        refbranchname, refpatchname = parse_rev(patchname)
>> +        if refpatchname:
>
> The corresponding TODO comment now would be that pick should be able
> to pick patches from the past, from the stack log.

How would the syntax look like?

-- 
Catalin