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.
> +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?
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.
> + # 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.
> +current one. The bottom of a patch is accessible with the
> +'[<branch>:]<patch>]^' format."""
You have an extra ] here.
> -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.)
> -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.
--
Karl Hasselström, kha@xxxxxxxxxxx
www.treskal.com/kalle
--
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
