2008/6/14 Karl Hasselström <kha@xxxxxxxxxxx>:
> On 2008-06-14 08:28:33 +0100, Catalin Marinas wrote:
>
>> The new scheme allows '[<branch>:]<patch>' and '[<branch>:]{base}'
>> (the latter showing the base of a stack). The former format allows
>> symbols like ^, ^{...} etc.
>
> I like your choices.
I think the initial idea belongs to Yann.
>> +def git_sha1(repository, branch, name):
>> + """Return the SHA1 value if 'name' is a patch name or Git commit.
>> + The patch names allowed are in the form '<branch>:<patch>' and can be
>> + followed by standard symbols used by git-rev-parse. If <patch> is '{base}',
>> + it represents the bottom of the stack.
>> + """
>
> Why not return the Commit directly, and let the caller extract its
> sha1 if that's what it wants?
OK, I was thinking about that. I'll convert it to git_commit(...) and
return a commit since I think it's only "stg id" that needs the SHA1
value.
> You don't remove the old parse_rev() and git_id(), and particularly
> the latter has a lot of callers. Meaning that the rest of StGit still
> speaks the old syntax.
I thought about removing them when we convert the commands to the new
infrastructure. In the meantime, I can rewrite git_id to use
git_commit directly. The parse_rev is only used by the 'pick' command
(and git_id). I'll have a look at these functions.
>> + # Try a Git commit first
>> + try:
>> + return repository.rev_parse(name, discard_stderr = True).sha1
>> + except libgit.RepositoryException:
>> + pass
>
> What if you have a branch or tag with the same name as a patch? This
> will prefer the branch, which might not be the best choice.
I can swap them and have the patch as preferred. People can be more
precise if they want the branch or tag by passing heads/... or
tags/...
>> -directory = DirectoryHasRepository()
>> +directory = common.DirectoryHasRepositoryLib()
>> options = [make_option('-b', '--branch',
>> help = 'use BRANCH instead of the default one')]
>
> Couldn't we kill this option? (And in the process, the branch argument
> to git_sha1.)
No problem with the option but I would like to keep the branch
argument to git_sha1. There might be cases where it is used like
picking multiple patches and I only specify one -B option to 'pick'. I
don't want to build the qualified patch name for every patch. I can
change the prototype of git_sha1 though (to git_commit):
def git_commit(name, repository, branch = None):
and branch needs to be passed explicitly and used instead of the
default one (when the patch name doesn't contain any).
>> -test_expect_success 'Try new form of id with slashy branch' \
>> +test_expect_success 'Try new id with slashy branch' \
>
> Strictly speaking, this isn't so new anymore.
The id format is new.
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
