In D12627#193347, @mharbison72 wrote:

In D12627#193334, @martinvonz wrote:

I *think* you're supposed to open a "merge request" on https://foss.heptapod.net/mercurial/mercurial-devel these days. Right, @Alphare?

I didn't think we made the switch yet, but I'm fine doing that as a pilot. (I ended up amending a commit comment typo since the last push, so it will need to run CI again.)

I *think* you're supposed to open a "merge request" on https://foss.heptapod.net/mercurial/mercurial-devel these days. Right, @Alphare?

Thanks for fixing! I'll apply this on stable.

For the record, this has been broken since 877d7e1

In D12582#193131, @Alphare wrote:

Unfortunately, this does not apply anymore. Sorry for the delay in review, it's been really packed lately.

In D12603#193012, @durin42 wrote:

In D12603#193005, @martinvonz wrote:

I agree, except that I've seen it crash sometimes. I wrote a little script to find such cases now. Here's an example: hg diff --change 7c2921a6. So we need to fix that first. Sorry I never bothered to report a bug for it.

That doesn't crash for me! Even HGRCPATH=/dev/null hg diff --change 7c2921a6 --config diff.merge=1 works...can you try and figure out why you're seeing a crash?

I agree, except that I've seen it crash sometimes. I wrote a little script to find such cases now. Here's an example: hg diff --change 7c2921a6. So we need to fix that first. Sorry I never bothered to report a bug for it.

In D12580#192736, @Alphare wrote:

In D12580#192721, @martinvonz wrote:

It looks like the tests from your other long series need to be updated in this patch (they don't compile with this patch applied).

You may have missed my first comment here then. This patch is destined for stable where it should apply cleanly, only the merge will need to be adapted .

It looks like the tests from your other long series need to be updated in this patch (they don't compile with this patch applied).

In D12541#192411, @Alphare wrote:

It is, thank you!

Is this series ready to be queued now?

In D12548#192301, @Alphare wrote:

The stack needs rebase apparently

In D12546#192136, @Alphare wrote:

In D12546#192112, @martinvonz wrote:

• I was thinking of moving the conditions further up the stack, to the repo level. If we do that, then the repo might be a natural place to extract a function. Or maybe I'll put it in nodemap.rs as you said. I'll update this patch a bit later.

Either seem fine to me.

In D12546#192085, @Alphare wrote:

While I think you're right about separating the revlogs from the repo, the repeated logic should probably be factored into a function somewhere. Why not in nodemap.rs?

In D12442#191985, @martinvonz wrote:

In D12442#191910, @Alphare wrote:

In D12442#191893, @martinvonz wrote:

That's actually related to another series I had not sent yet, which is about removing the &Repo input to Revlog::open(). It would be much easier to write tests if we didn't have to create a Repo instance.

I see no issue for doing this, it's probably a good idea.

I've inserted those patches in this series.

In D12442#191910, @Alphare wrote:

In D12442#191893, @martinvonz wrote:

That's actually related to another series I had not sent yet, which is about removing the &Repo input to Revlog::open(). It would be much easier to write tests if we didn't have to create a Repo instance.

I see no issue for doing this, it's probably a good idea.