William Giokas wrote:
> On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote:

> >     Why do we "import changegroup" unconditionally, even though it
> >     is only used in the new codepath meant only for version 3.0 or
> >     higher, not inside the "if" block that decides if we need that
> >     module? 

> changegroup is prefectly /okay/ to import unconditionally, though as you
> say it will never be used.

As you say, it's perfectly OK.

> We can also be even more explicit with what we import by doing something
> like::
> 
>   try:
>       from mercurial.changegroup import getbundle
> 
>   except ImportError:
>       def getbundle(__empty__, **kwargs):
>           return repo.getbundle(**kwargs)

We could try that, but that would assume we want to maintain getbundle()
for the long run, and I personally don't want to do that. I would rather
contact the Mercurial developers about ways in which the push() method
can be improved so we don't need to have our own version. Hopefully
after it's improved we wouldn't have to call getbundle().

Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at
some point we would want to remove the hacks for older versions. When we
do so we would want the import to remain unconditionally, and remove the
'check_version(3, 0)' which is already helping to explain what the code
is for without the need of comments.

> I was really sad to see that, and didn't have time to really look at it
> because of work and other projects, but I hope this presents a better
> solution than the current patch.

Either way Junio doesn't maintain this code, I do. And it's not
maintained in git.git, git's maintained out-of-tree (thanks to Junio's
decisions).

So please post your suggestions and patches to git...@googlegroups.com,
and use the latest code at https://github.com/felipec/git-remote-hg.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to