D6734: git: skeleton of a new extension to _directly_ operate on git repos

durin42 (Augie Fackler) phabricator at mercurial-scm.org
Wed Mar 4 18:47:57 UTC 2020


durin42 added inline comments.

INLINE COMMENTS

> martinvonz wrote in __init__.py:38-39
> What blocks that?

I've added a TODO.md that documents what needs to happen here (missing an interface.)

> martinvonz wrote in __init__.py:68-72
> Or maybe pygit2 takes care of locking while it updates? So I wonder if this is fine the way it is. No action required.

Noted this in the TODO.md

> martinvonz wrote in __init__.py:125
> nit: drop the leading `\n` and teach people to include newline at EOF instead?

This was intentional: I don't want to take a valid-but-missing-trailing-newline `.git/info/exclude` and blindly write a `.hg` at the end of an existing line. Are you saying the paranoia feels misguided?

> martinvonz wrote in __init__.py:126-129
> Did you intend to call the file `requires` and not need `this-is-git`? I think this extension should also register with `featuresetupfuncs`.

It honestly didn't occur to me to use the presence of `git` in `requires` to trigger the "this is a git repo" behavior. Should I add a TODO about that?

> martinvonz wrote in gitlog.py:29
> Could we also get `__iter__`? We can of course add that later, but maybe it seems easy to add anyway (`revlog.py` has `return iter(pycompat.xrange(len(self)))`).
> 
> Maybe also copy the following from `revlog.py`?
> 
>   def tiprev(self):
>       return len(self.index) - 1 # well, use "len(self)" here, I guess
>   
>   def tip(self):
>       return self.node(self.tiprev())
>   
>   def revs(self, start=0, stop=None):
>       """iterate over all rev in this revlog (from start to stop)"""
>       return storageutil.iterrevs(len(self), start=start, stop=stop)

I'm hoping we can only implement __iter__ on changelog, not on baselog. Ditto for tip and revs, but I was going to block that on the interface definition (I haven't yet seen anything that wants these methods...)

> martinvonz wrote in gitlog.py:150
> Will the `?` be replaced by `abc123%` or `b'abc123%'` on py3? (Same applies further down.)

It should be the former. I've actually been developing this extension exclusively on Python 3, so the tests already pass on 3.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6734/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6734

To: durin42, #hg-reviewers
Cc: martinvonz, sluongng, tom.prince, sheehan, rom1dep, JordiGH, hollisb, mjpieters, mercurial-devel


More information about the Mercurial-devel mailing list