Re: Kill off contrib/simplemerge?

2017-08-15 Thread Phillip Cohen
:56 PM, Martin von Zweigbergk via Mercurial-devel wrote: > What is the script used for? Is it used by "hg merge/rebase/etc"? Only > if you pass --tool ? Not at all? > > On Tue, Aug 15, 2017 at 2:52 PM, Phillip Cohen wrote: >> Hello, >> >> During some of the wor

Kill off contrib/simplemerge?

2017-08-15 Thread Phillip Cohen
Hello, During some of the work to support in-memory merge, the `contrib/simplemerge` script (not to be confused with simplemerge.py in the mercurial package) has come up as a bit of a roadblock that makes existing code messier, and I was wondering if anyone would object if we removed it. This is

Re: Experimenting with Phabricator for reviews

2017-07-15 Thread Phillip Cohen
I think (and it seems like most people agree here) that we should take an axe to Phabricator's default e-mails and strip out the cruft. I don't think that'd be impossibly hard to do (says somebody about to go on PTO), either by YOLO patching the code for now or writing an extension, or just using a

Re: Community video chats?

2017-07-15 Thread Phillip Cohen
Tue, 2017-07-11 at 16:08 -0400, Augie Fackler wrote: >>>>> On Jul 11, 2017, at 15:47, Phillip Cohen >>>>> wrote: >>>>> >>>>> Great, glad there's interest. >>>>> >>>>> How would Thursday at 10am PS

Re: Experimenting with Phabricator for reviews

2017-07-12 Thread Phillip Cohen
> Unless you're talking about Phabricator's webui. Then I don't know because I don't use it. I was, sorry for the ambiguity. On Wed, Jul 12, 2017 at 1:42 PM, Sean Farley wrote: > > Phillip Cohen writes: > >> I'm using the webui, though, so it all loo

Re: Experimenting with Phabricator for reviews

2017-07-12 Thread Phillip Cohen
I'm using the webui, though, so it all looks the same. On Wed, Jul 12, 2017 at 1:35 PM, Sean Farley wrote: > > Phillip Cohen writes: > >>> Also: Jun has suggested to me that we should avoid using "Accept >>> Commit", because that should be a state appli

Re: Experimenting with Phabricator for reviews

2017-07-12 Thread Phillip Cohen
or extensions. Looking upstream (https://secure.phabricator.com/T5244, https://secure.phabricator.com/D9342), it looks like this is something they don't want to support. But curiously, in the latter link, the creator suggested just patching it locally. On Wed, Jul 12, 2017 at 12:44 PM, Durham Goode wrote: >

Re: Experimenting with Phabricator for reviews

2017-07-12 Thread Phillip Cohen
> Also: Jun has suggested to me that we should avoid using "Accept > Commit", because that should be a state applied only when a commit is > queued to `hg-committed` (either by the reviewer or by a future bot). > Once the commit goes to `hg` it should be automatically closed (this > should happen t

Re: Experimenting with Phabricator for reviews

2017-07-11 Thread Phillip Cohen
Also: Jun has suggested to me that we should avoid using "Accept Commit", because that should be a state applied only when a commit is queued to `hg-committed` (either by the reviewer or by a future bot). Once the commit goes to `hg` it should be automatically closed (this should happen today out o

Re: Community video chats?

2017-07-11 Thread Phillip Cohen
Great, glad there's interest. How would Thursday at 10am PST / 1pm EST / 6pm UTC work? This is early enough for London to join, but not too early that California won't. Friday mornings are less desirable (because that's Friday evening in London) but Monday and Tuesday morning could work as well.

Re: Experimenting with Phabricator for reviews

2017-07-11 Thread Phillip Cohen
BTW: Phabricator doesn't seem to have official mobile clients, but their mobile site is pretty good, and lets you read diffs and send quick replies on the go. Handy if you used to use e-mail for this purpose. On iOS you can just add https://phab.mercurial-scm.org/differential/ as a home screen boo

Re: [PATCH 07 of 10] phabricator: try to fetch differential revisions in batch

2017-07-04 Thread Phillip Cohen
> when fetching D59 as a stack, previous IDs like D51, D52, D53, ..., D58 are likely belonging to a same stack so just fetch them as well I look forward to the day when we have enough contributors that this is unlikely to be true :) On Tue, Jul 4, 2017 at 6:58 PM, Jun Wu wrote: > # HG changeset

Re: [PATCH 1 of 5] show: tweak warning message

2017-07-04 Thread Phillip Cohen
Is there any consensus on whether to use "working directory parent" vs. "working copy parent"? I see both used in the codebase (though more occurrences of "working directory parent"). On Tue, Jul 4, 2017 at 6:29 AM, Yuya Nishihara wrote: > On Mon, 03 Jul 2017 21:32:37 -0700, Gregory Szorc wrote:

Re: [PATCH 4 of 5 V2] simplemerge: use contexts to read file data from, if passed

2017-07-04 Thread Phillip Cohen
I was going to, just to keep our options open, but if it's undesirable I can close that off and have contrib/simplemerge pass a fake context and labels. > > - warning: $TESTTMP/bigfile-repo/stuff/maybelarge.dat looks like a binary > > file. (glob) > > + warning: stuff/maybelarge.dat looks like

Re: [PATCH 10 of 10] phabricator: do not read a same revision twice

2017-07-04 Thread Phillip Cohen
> Hopefully, comments from phabricator will get mirrored to the list as well so that subscribers can see the reviews +1 on this. It should be pretty easy to configure on the Phab side. On Tue, Jul 4, 2017 at 7:41 PM, Sean Farley wrote: > > Jun Wu writes: > >> # HG changeset patch >> # User Jun

Re: [PATCH 1 of 5] simplemerge: extract verifytext as a helper function

2017-07-04 Thread Phillip Cohen
My fault :) I'll resend it. On Tue, Jul 4, 2017 at 7:31 AM, Yuya Nishihara wrote: > On Wed, 31 Dec 1969 16:03:11 -0800, Phil Cohen wrote: >> # HG changeset patch >> # User Phil Cohen >> # Date 200 28800 >> # Wed Dec 31 16:03:20 1969 -0800 > > An email from history. :) > Can you resend updat

Re: [PATCH 2 of 3] phabricator: add phabsend command to send a stack

2017-07-03 Thread Phillip Cohen
> +""" > +revs = list(revs) + opts.get('rev', []) > +revs = scmutil.revrange(repo, revs) > + I'd add something like this here: if not revs: raise error.Abort(_('must specify at least one changeset with -r')) On Sun, Jul 2, 2017 at 8:10 PM, Jun Wu wrote: > # HG changeset patch >

Re: [PATCH 1 of 3] phabricator: add a contrib script

2017-07-02 Thread Phillip Cohen
This is a great idea, thanks for implementing it. :) On Sun, Jul 2, 2017 at 9:42 PM, Siddharth Agarwal wrote: > On 7/2/17 8:10 PM, Jun Wu wrote: >> >> # HG changeset patch >> # User Jun Wu >> # Date 1499051289 25200 >> # Sun Jul 02 20:08:09 2017 -0700 >> # Node ID dde88a5ead698208226301a62a

Re: [PATCH] filemerge: convert a couple of wvfs calls in internal mergetools to contexts

2017-06-28 Thread Phillip Cohen
t over-complicating things. On Mon, Jun 26, 2017 at 11:12 PM, Phillip Cohen wrote: >> I discussed with Sidd about just having the getters > raise RuntimeErrors after a mutator has been called, but we actually call > isabsent() in merge.py after running the internal merge tools. > > It look

Re: [PATCH] filemerge: convert a couple of wvfs calls in internal mergetools to contexts

2017-06-26 Thread Phillip Cohen
> I discussed with Sidd about just having the getters raise RuntimeErrors after a mutator has been called, but we actually call isabsent() in merge.py after running the internal merge tools. It looks like in the test suite, we only call isabsent() after calling remove(). So perhaps a reasonable op

Re: [PATCH 4 of 4 hyperblame] annotate: add a new experimental --skip option to skip revs

2017-05-25 Thread Phillip Cohen
> 1. Should we call it --skip, --ignore or something else? "--skip" feels more intuitive to me, but IANANE. On Thu, May 25, 2017 at 4:49 PM, Siddharth Agarwal wrote: > On 5/24/17 7:39 PM, Siddharth Agarwal wrote: >> >> # HG changeset patch >> # User Siddharth Agarwal >> # Date 1495679973 25200

Re: Coding style change proposals

2017-05-13 Thread Phillip Cohen
+1 to #1, +2 to #2, +0.5 to #3. Excited about this progression of the codebase. Thanks for sending this out. I'd like to humbly suggest: classes in their own files, at least for important classes? Or, if not that, perhaps some basic ordering rules for files, like that classes must come first? Som

Re: New Reviewers!

2017-05-13 Thread Phillip Cohen
👏 On Fri, May 12, 2017 at 7:47 PM, Augie Fackler wrote: > Greetings! > > The steering committee has granted push access (but not accept access) to > Sean Farley and Gregory Szorc. They can both push patches, but one of the > existing committers will still need to approve patches before they lan

Re: [PATCH 1 of 2 STABLE] diffhelpers: add canstripcr=True to fix_newline

2017-04-26 Thread Phillip Cohen
I had chatted with Jun about this earlier -- if the most elegant way is simply to wait for his patch or Yuya's to improve the C module versioning, which would have to wait until after the freeze anyway, then this would have to wait too. I'm fine with that since it seems the most reasonable approach

Re: [PATCH 1 of 2 STABLE] diffhelpers: add canstripcr=True to fix_newline

2017-04-25 Thread Phillip Cohen
> Maybe it doesn't check if the line has no extra character? Do you mean that we no longer check that the "\ No newline at end of file" line has a newline? On Tue, Apr 25, 2017 at 11:23 AM, Jun Wu wrote: > Excerpts from Yuya Nishihara's message of 2017-04-25 22:56:13 +0900: >> This will break C

Re: [PATCH 1 of 2 STABLE] diffhelpers: add canstripcr=True to fix_newline

2017-04-24 Thread Phillip Cohen
> Very minor nit: is that an extra space in front of '{'? Not sure about our C-style. Eep, you're right. On Mon, Apr 24, 2017 at 2:45 PM, Sean Farley wrote: > Jun Wu writes: > >> This fixes a 4-year bug. I think it is worth being merged. > > I would agree. The patch seems fine, safety-wise (but

Re: [PATCH V2] merge: add `internal:dumpjson` tool to `resolve`, which outputs conflict state

2017-03-22 Thread Phillip Cohen
> It doesn't make sense to use the formatter to dump a single object. Perhaps > formatter._jsonifyobj() can be a public utility function. That's a good idea. If there are no objections I'll send that out, either as the first change in the next version or a isolated change (probably the former so

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

2017-03-21 Thread Phillip Cohen
A tuple or subobject works for me. On Tue, Mar 21, 2017 at 1:45 PM, Jun Wu wrote: > Excerpts from Ryan McElroy's message of 2017-03-21 20:32:56 +: >> >> On 3/21/17 7:34 PM, Jun Wu wrote: >> > Excerpts from Phillip Cohen's message of 2017-03-21 12:21:33 -0700: >> >>> Have you actually tried if

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

2017-03-21 Thread Phillip Cohen
> Have you actually tried if "commandname" is the command name after resolving > alias? It is, for command aliases. For example `hg sf` will correctly return `absorb`. On Tue, Mar 21, 2017 at 12:18 PM, Jun Wu wrote: > > Excerpts from Ryan McElroy's message of 2017-03-21 18:56:19 +: > > > > O

Re: [PATCH 1 of 4 V3] filemerge: add `summarize()`

2017-03-20 Thread Phillip Cohen
> Why do we need a copy here? Is it just to protect a caller from accidents like munging the returned dict and accidentally having it munge both output and local? Let's add a comment about why we're doing this copy. It's because we add the 'path' key to `output` after this, to show to the user whe

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

2017-03-20 Thread Phillip Cohen
> Instead of having this information inside of a lock, what if every process that started up in a repo tried to create a file in .hg/processes/ (or something) of the form "info.". Then you wouldn't need to pass this data through to the lock command, wouldn't need to plumb it through the ui class, a

Re: [PATCH 1 of 3] dispatch: store args and command run on ui

2017-03-20 Thread Phillip Cohen
Why not `ui.argv`?, OOC? On Mon, Mar 20, 2017 at 9:16 AM, Jun Wu wrote: > Excerpts from Ryan McElroy's message of 2017-03-20 11:43:22 +: > > I think Jun needs to weigh in on this since the ui object is something > > he's wanted to make immutable for a while and this doesn't seem to help > >