# User Details

User Since
Jun 28 2017, 5:28 PM (145 w, 15 h)

# Tue, Apr 7

I would prefer you wait for planned change to land before you do further change to the file. The coming changes fixes bug and I rather not have to fix too many conflict while doing so.

I already pushed them, I will resolve conflicts on those couple of patches and update them on heptapod.

tests: move verification closer to setup in test-copies-chain-merge.t
tests: collect all branch creation in one place in test-copies-chain-merge.t

# Fri, Apr 3

rebase: don't create merge when continuing rebase interrupted by old hg
tests: demonstrate how continuing rebase after upgrade can result in merge

# Thu, Apr 2

D8258: copies-tests: remove spurious ] in the template is now accepted and ready to land.

This one is trivial to move ahead of the other two patches so I'll queue it and fix any conflicts in flight.

# Wed, Apr 1

martinvonz added a comment to D7827: rebase: don't use rebased node as dirstate p2 (BC).

Heads up that we ran into a fun bug that was probably caused by this patch. I think what happened was this:

# Thu, Mar 26

py3: require values in changelog extras to be bytes
py3: make setup.py's hgcommand() consistently return bytes

# Wed, Mar 25

pvec: drop an unused from __future__ import division
py3: use integer division in histedit
shelve: split up dounshelve() in unshelvecmd() and _dounshelve()
martinvonz added inline comments to D8324: py3: use integer division in histedit.
martinvonz updated the diff for D8324: py3: use integer division in histedit.

# Fri, Mar 20

Since narrow is still experimental, I don't think we should try too hard for backward compatibility. We could introduce a new end-point for a new encoding and drop the old one in a couple of version.

+0, honestly. I won't require it, but I'd really rather we shaved this yak _now_ rather than when narrow has even more users.

I'm getting a bit frustrated with how much time I've spent on this, made worse by the fact that I agree with everything everyone's saying and so it's not like I'm frustrated at the review process, just how slow I've been at accomplishing this.
So, before I go down another rabbit hole, here's what I'm thinking:

• Server emits a new capability narrow-exp-1-escaped (in addition to the current narrow-exp-1, this is not replacing the existing capability)
martinvonz updated subscribers of D8190: nodemap: test that concurrent process don't see the pending transaction.

The parent patch (D8189) was a bit controversial. We don't need that patch if we apply the following patch on top of this one (making this test sleep-free):

# Thu, Mar 19

This is pretty ugly and it doesn't seem that the next patch depends on it. You said you'll soon clean it up anyway, so I wonder if should just wait for the better solution instead. It doesn't seem like this fixes a serious bug so we have to rush it. Thoughts?

My initial motivation to rush the ugly way was "getting the behavior right to compare with the changeset centric one and being able to test performance improvement for the changeset centric one while having access to a specific repository". However, cancelling of all travel has cancelled the window to access that repo. I'll resubmit a cleaner versions soon.
since you did not commented on it, I assume the new behavior is fine by you, right?

martinvonz added a comment to D8289: resolve: add a --clear option for clearing the merge state.

I like the idea. IIRC, Ryan from FB hit similar issues in a sprint some years ago and came up with hg up --finish or something like that.
Maybe we should not let user clear the mergestate and suggest continue/<cmd-name> --continue if it's not result of update command. Thoughts?

Ho, that's a good idea. It looks like hg update --merge is the only command that do not have --continue support. So instead of adding a whole new flag and action to this exception, removing the exception seems like a better move. What do you think @martinvonz ?

fix: add a -s option to format a revision and its descendants
fix: move handling of --all into getrevstofix() for consistency

# Wed, Mar 18

martinvonz updated the diff for D8288: fix: mark -r as advanced.

# Tue, Mar 17

martinvonz updated the summary of D8289: resolve: add a --clear option for clearing the merge state.

# Mon, Mar 16

fix: refactor getrevstofix() to define revisions first, then validate them
fix: disallow hg fix --all --working-dir
martinvonz updated the diff for D8288: fix: mark -r as advanced.
martinvonz added a comment to D8284: fix: disallow hg fix --all --working-dir.

If --working-dir and --all are redundant, I don't see anyharm in allowing both to be passed.

The idea was to inform users that they're doing something that's a little weird, in case they were hoping for it to do something else. I don't care much and I'm fine with dropping this patch if that's the consensus.

If the intend is to inform, maybe we could issue a warning?

martinvonz added a comment to D8284: fix: disallow hg fix --all --working-dir.

If --working-dir and --all are redundant, I don't see anyharm in allowing both to be passed.

# Sat, Mar 14

tests: simplify test-fix-topology.t slightly by using a (case !)
tests: fix rebase test broken by earlier cleanup
rebase: accept multiple --base arguments (BC)
rebase: accept multiple --source arguments (BC)
rebase: mention -r argument in synopsis
rebase: remove unused defaults argument values from _definedestmap()

# Fri, Mar 13

martinvonz added a comment to D8293: rebase: accept multiple --base arguments (BC).

I've updated the synopsis (also added another patch in the series for adding -r to the synopsis).

martinvonz updated the diff for D8292: rebase: accept multiple --source arguments (BC).
martinvonz updated the diff for D8293: rebase: accept multiple --base arguments (BC).
martinvonz retitled D8295: rebase: mention -r argument in synopsis from rebase: mention -r argument in synposis to rebase: mention -r argument in synopsis.
martinvonz retitled D8293: rebase: accept multiple --base arguments (BC) from rebase: accept multiple --base arguments to rebase: accept multiple --base arguments (BC).
D8136: phabricator: add a phabimport command is now accepted and ready to land.
martinvonz updated the diff for D8288: fix: mark -r as advanced.
martinvonz added inline comments to D8287: fix: add a -s option to format a revision and its descendants.
martinvonz updated the diff for D8288: fix: mark -r as advanced.
martinvonz updated the summary of D8282: tests: consistently put #testcases at beginning of file.
martinvonz added a comment to D8282: tests: consistently put #testcases at beginning of file.

Can you drop your change to tests/test-push-race.t from this patch ?

martinvonz added a comment to D8282: tests: consistently put #testcases at beginning of file.

That's a good point, doing it for #require too would be more consistent.
I would make a difference between test that do not make too much of an effort to have a title + early documentation and the one who actually make effort to have a formal format for their title and documentation (the one with =====\ntitle\n===== or title\n=====).
I would be fine with having the requires/testcases right after the title when it make senses. I think there are some instance where the variants are formally documented, and the #testcase block is part of that. It might make sense to keep them at that location.

martinvonz added a comment to D8282: tests: consistently put #testcases at beginning of file.

(like imports and includes in other languages)

Precisely, import and includes usually comes after the initial module licence, title and documentation. So I would like the testcase declaration to comes after the initial module title (and maybe doc).

The Windows path changes seem like a good idea.
Would quoting paths with commas eliminate the need for custom escaping? I don't feel strongly about it, but custom escaping always feels weird to me. (I fact, a coworker did some homebrew escaping for CSV files a few days ago, but I forget how it ultimately ended up.)

Let me play with that a bit, I think it'll work and be detectable on the server since the first character can't currently be a double-quote, so there wouldn't really be any BC issues aside from the pconvert (which wouldn't be as important anymore, but still probably a good idea?)

I haven't played with narrow yet, so I'm not sure of the context. Are these user input paths that would end up being ignored/rejected if a Windows user used path\to\file when talking to a Unix server? Or are these stored in a tracked file? (Which I think could still cause problems.) I can't think of a good reason to stay inconsistent, and it is still experimental, so it still seems like a good idea while we still can fix it.

martinvonz added a comment to D8282: tests: consistently put #testcases at beginning of file.

I very much agree with Matt

martinvonz added inline comments to D8282: tests: consistently put #testcases at beginning of file.