Page MenuHomePhabricator

git: key off `git` in .hg/requires rather than separate file
ClosedPublic

Authored by durin42 on Sat, Mar 7, 5:57 PM.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

durin42 created this revision.Sat, Mar 7, 5:57 PM
mharbison72 added inline comments.
hgext/git/__init__.py
111

Shouldn't it be an error if git is in requirements, but .git doesn't exist? Or is there something in the original function that will raise an error in this case?

durin42 updated this revision to Diff 20664.Tue, Mar 10, 12:31 PM
durin42 updated this revision to Diff 20665.Tue, Mar 10, 12:35 PM
durin42 marked an inline comment as done.Tue, Mar 10, 12:35 PM
durin42 added inline comments.
hgext/git/__init__.py
111

Good catch! Yes, that makes sense.

martinvonz added inline comments.
hgext/git/__init__.py
89–95

Why not use .hg/requires?

durin42 marked an inline comment as done.Tue, Mar 10, 12:46 PM
durin42 updated this revision to Diff 20666.
durin42 updated this revision to Diff 20667.Tue, Mar 10, 12:59 PM
pulkit added a subscriber: pulkit.Tue, Mar 10, 1:03 PM
pulkit added inline comments.
hgext/git/dirstate.py
79 ↗(On Diff #20667)

looks like unrelated change which should be in separate patch?

durin42 added inline comments.Tue, Mar 10, 1:04 PM
hgext/git/__init__.py
89–95

Because I have the dumb. Will fix shortly.

hgext/git/dirstate.py
79 ↗(On Diff #20667)

Sigh, you're right, and I'm being lazy. I'll split this out in a few minutes.

martinvonz added inline comments.Tue, Mar 10, 1:08 PM
hgext/git/__init__.py
89–96

is this different from if b'git' in requirements?

tests/test-git-interop.t
5–11

$ prefix on these too since you're cleaning up anyway?

durin42 marked 4 inline comments as done.Tue, Mar 10, 1:14 PM
durin42 added inline comments.
tests/test-git-interop.t
5–11

My understanding is that the > will cause the lines to all happen in the same shell invocation, whereas $ will do separate evaluations of each line, so it's (more or less) intentional. I believe it was a bug that the first line had a > instead of a $ (and since I was rearranging the intro stanza I fixed it up).

I can revert that top byte change if you'd prefer.

durin42 marked an inline comment as done.Tue, Mar 10, 1:15 PM
durin42 retitled this revision from git: key off `git` in .hg/requirements rather than separate file to git: key off `git` in .hg/requires rather than separate file.
durin42 updated this revision to Diff 20669.
martinvonz added inline comments.Tue, Mar 10, 1:19 PM
tests/test-git-interop.t
5–11

I'm fine with leaving it as it is.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.