Patchwork [2,of,2,v2] rebase: add flag to require destination

login
register
mail settings
Submitter Ryan McElroy
Date March 15, 2017, 12:56 a.m.
Message ID <8d8b783803f43d5e2d86.1489539376@devbig314.prn1.facebook.com>
Download mbox | patch
Permalink /patch/19343/
State Accepted
Headers show

Comments

Ryan McElroy - March 15, 2017, 12:56 a.m.
# HG changeset patch
# User Ryan McElroy <rmcelroy@fb.com>
# Date 1489538624 25200
#      Tue Mar 14 17:43:44 2017 -0700
# Node ID 8d8b783803f43d5e2d86916c39e9554139752fe6
# Parent  2dc26c57e60e7e7bf46a276e8a498a9746bd9271
rebase: add flag to require destination

In some mercurial workflows, the default destination for rebase does not
always work well and can lead to confusing behavior. With this flag enabled,
every rebase command will require passing an explicit destination, eliminating
this confusion.
David Soria Parra - March 15, 2017, 3:26 a.m.
On Tue, Mar 14, 2017 at 05:56:16PM -0700, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy <rmcelroy@fb.com>
> # Date 1489538624 25200
> #      Tue Mar 14 17:43:44 2017 -0700
> # Node ID 8d8b783803f43d5e2d86916c39e9554139752fe6
> # Parent  2dc26c57e60e7e7bf46a276e8a498a9746bd9271
> rebase: add flag to require destination
> 

This looks good to me. I was wondering if we want to provide separate knobs for
these commands which might lead to config overhead or provide more comprehensive
"ui" improvement knobs such as "commands.requiredest" to move people to a better
model in logical steps.

e.g. I am a user who likes a slightly enhanced user experience. ui.compat= is a
bit too much for me, but update destinations is a good idea. Do i have to find
all places where we use destinations to update or do I want to select a logical
step?

I personally think while fine granualar steps are nice, I'd probably lean
towards logical steps as it provides a more consistent behavior for users (e.g.
assume an extension Y that we don't know of can opt into using
"commands.requiredest", which at the moment it cannot unless it depends on
"commands.update.requiredest" which is missleading.
Ryan McElroy - March 16, 2017, 1:55 a.m.
On 3/14/17 8:26 PM, David Soria Parra wrote:
> On Tue, Mar 14, 2017 at 05:56:16PM -0700, Ryan McElroy wrote:
>> # HG changeset patch
>> # User Ryan McElroy <rmcelroy@fb.com>
>> # Date 1489538624 25200
>> #      Tue Mar 14 17:43:44 2017 -0700
>> # Node ID 8d8b783803f43d5e2d86916c39e9554139752fe6
>> # Parent  2dc26c57e60e7e7bf46a276e8a498a9746bd9271
>> rebase: add flag to require destination
>>
> This looks good to me. I was wondering if we want to provide separate knobs for
> these commands which might lead to config overhead or provide more comprehensive
> "ui" improvement knobs such as "commands.requiredest" to move people to a better
> model in logical steps.
>
> e.g. I am a user who likes a slightly enhanced user experience. ui.compat= is a
> bit too much for me, but update destinations is a good idea. Do i have to find
> all places where we use destinations to update or do I want to select a logical
> step?
>
> I personally think while fine granualar steps are nice, I'd probably lean
> towards logical steps as it provides a more consistent behavior for users (e.g.
> assume an extension Y that we don't know of can opt into using
> "commands.requiredest", which at the moment it cannot unless it depends on
> "commands.update.requiredest" which is missleading.

I'm not against this direction, but I think what I have proposed here is 
stillt he first right step. Once we have a bunch of granular knobs like 
these ones, we can then work towards "multiknobs" when we have the 
config registry concept to tie options together more, and then the 
compatibility levels are just the biggest "multiknobs".
Ryan McElroy - March 20, 2017, 3:17 p.m.
Any objections here?


On 3/16/17 1:55 AM, Ryan McElroy wrote:
>
>
> On 3/14/17 8:26 PM, David Soria Parra wrote:
>> On Tue, Mar 14, 2017 at 05:56:16PM -0700, Ryan McElroy wrote:
>>> # HG changeset patch
>>> # User Ryan McElroy <rmcelroy@fb.com>
>>> # Date 1489538624 25200
>>> #      Tue Mar 14 17:43:44 2017 -0700
>>> # Node ID 8d8b783803f43d5e2d86916c39e9554139752fe6
>>> # Parent  2dc26c57e60e7e7bf46a276e8a498a9746bd9271
>>> rebase: add flag to require destination
>>>
>> This looks good to me. I was wondering if we want to provide separate 
>> knobs for
>> these commands which might lead to config overhead or provide more 
>> comprehensive
>> "ui" improvement knobs such as "commands.requiredest" to move people 
>> to a better
>> model in logical steps.
>>
>> e.g. I am a user who likes a slightly enhanced user experience. 
>> ui.compat= is a
>> bit too much for me, but update destinations is a good idea. Do i 
>> have to find
>> all places where we use destinations to update or do I want to select 
>> a logical
>> step?
>>
>> I personally think while fine granualar steps are nice, I'd probably 
>> lean
>> towards logical steps as it provides a more consistent behavior for 
>> users (e.g.
>> assume an extension Y that we don't know of can opt into using
>> "commands.requiredest", which at the moment it cannot unless it 
>> depends on
>> "commands.update.requiredest" which is missleading.
>
> I'm not against this direction, but I think what I have proposed here 
> is stillt he first right step. Once we have a bunch of granular knobs 
> like these ones, we can then work towards "multiknobs" when we have 
> the config registry concept to tie options together more, and then the 
> compatibility levels are just the biggest "multiknobs".
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
Augie Fackler - March 21, 2017, 10:34 p.m.
On Mon, Mar 20, 2017 at 03:17:47PM +0000, Ryan McElroy wrote:
> Any objections here?

Hearing none, queued.

Do we still think that the 'compat' knob belongs here (as we thought
with [behavior]), or should it live under [ui]?

>
>
> On 3/16/17 1:55 AM, Ryan McElroy wrote:
> >
> >
> >On 3/14/17 8:26 PM, David Soria Parra wrote:
> >>On Tue, Mar 14, 2017 at 05:56:16PM -0700, Ryan McElroy wrote:
> >>># HG changeset patch
> >>># User Ryan McElroy <rmcelroy@fb.com>
> >>># Date 1489538624 25200
> >>>#      Tue Mar 14 17:43:44 2017 -0700
> >>># Node ID 8d8b783803f43d5e2d86916c39e9554139752fe6
> >>># Parent  2dc26c57e60e7e7bf46a276e8a498a9746bd9271
> >>>rebase: add flag to require destination
> >>>
> >>This looks good to me. I was wondering if we want to provide separate
> >>knobs for
> >>these commands which might lead to config overhead or provide more
> >>comprehensive
> >>"ui" improvement knobs such as "commands.requiredest" to move people to
> >>a better
> >>model in logical steps.
> >>
> >>e.g. I am a user who likes a slightly enhanced user experience.
> >>ui.compat= is a
> >>bit too much for me, but update destinations is a good idea. Do i have
> >>to find
> >>all places where we use destinations to update or do I want to select a
> >>logical
> >>step?
> >>
> >>I personally think while fine granualar steps are nice, I'd probably
> >>lean
> >>towards logical steps as it provides a more consistent behavior for
> >>users (e.g.
> >>assume an extension Y that we don't know of can opt into using
> >>"commands.requiredest", which at the moment it cannot unless it depends
> >>on
> >>"commands.update.requiredest" which is missleading.
> >
> >I'm not against this direction, but I think what I have proposed here is
> >stillt he first right step. Once we have a bunch of granular knobs like
> >these ones, we can then work towards "multiknobs" when we have the config
> >registry concept to tie options together more, and then the compatibility
> >levels are just the biggest "multiknobs".
> >
> >
> >_______________________________________________
> >Mercurial-devel mailing list
> >Mercurial-devel@mercurial-scm.org
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Gregory Szorc - March 22, 2017, 3:14 a.m.
On Mon, Mar 20, 2017 at 8:17 AM, Ryan McElroy <rm@fb.com> wrote:

> Any objections here?
>

No. I agree with your approach here. Implement the granular knobs first.
Then build the "multiknobs" later.



>
> On 3/16/17 1:55 AM, Ryan McElroy wrote:
>
>>
>>
>> On 3/14/17 8:26 PM, David Soria Parra wrote:
>>
>>> On Tue, Mar 14, 2017 at 05:56:16PM -0700, Ryan McElroy wrote:
>>>
>>>> # HG changeset patch
>>>> # User Ryan McElroy <rmcelroy@fb.com>
>>>> # Date 1489538624 25200
>>>> #      Tue Mar 14 17:43:44 2017 -0700
>>>> # Node ID 8d8b783803f43d5e2d86916c39e9554139752fe6
>>>> # Parent  2dc26c57e60e7e7bf46a276e8a498a9746bd9271
>>>> rebase: add flag to require destination
>>>>
>>>> This looks good to me. I was wondering if we want to provide separate
>>> knobs for
>>> these commands which might lead to config overhead or provide more
>>> comprehensive
>>> "ui" improvement knobs such as "commands.requiredest" to move people to
>>> a better
>>> model in logical steps.
>>>
>>> e.g. I am a user who likes a slightly enhanced user experience.
>>> ui.compat= is a
>>> bit too much for me, but update destinations is a good idea. Do i have
>>> to find
>>> all places where we use destinations to update or do I want to select a
>>> logical
>>> step?
>>>
>>> I personally think while fine granualar steps are nice, I'd probably lean
>>> towards logical steps as it provides a more consistent behavior for
>>> users (e.g.
>>> assume an extension Y that we don't know of can opt into using
>>> "commands.requiredest", which at the moment it cannot unless it depends
>>> on
>>> "commands.update.requiredest" which is missleading.
>>>
>>
>> I'm not against this direction, but I think what I have proposed here is
>> stillt he first right step. Once we have a bunch of granular knobs like
>> these ones, we can then work towards "multiknobs" when we have the config
>> registry concept to tie options together more, and then the compatibility
>> levels are just the biggest "multiknobs".
>>
>>
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -650,6 +650,16 @@  def rebase(ui, repo, **opts):
 
           hg rebase -r "branch(featureX)" -d 1.3 --keepbranches
 
+    Configuration Options:
+
+    You can make rebase require a destination if you set the following config
+    option:
+
+      [commands]
+      rebase.requiredest = False
+
+    Return Values:
+
     Returns 0 on success, 1 if nothing to rebase or there are
     unresolved conflicts.
 
@@ -663,6 +673,12 @@  def rebase(ui, repo, **opts):
 
         # Validate input and define rebasing points
         destf = opts.get('dest', None)
+
+        if ui.config('commands', 'rebase.requiredest', False):
+            if not destf:
+                raise error.Abort(_('you must specify a destination'),
+                                  hint=_('use: hg rebase -d REV'))
+
         srcf = opts.get('source', None)
         basef = opts.get('base', None)
         revf = opts.get('rev', [])
diff --git a/tests/test-rebase-base.t b/tests/test-rebase-base.t
--- a/tests/test-rebase-base.t
+++ b/tests/test-rebase-base.t
@@ -391,3 +391,25 @@  Multiple roots. Two children share two p
    /
   o  0: A
   
+Require a destination
+  $ cat >> $HGRCPATH <<EOF
+  > [commands]
+  > rebase.requiredest = True
+  > EOF
+  $ hg init repo
+  $ cd repo
+  $ echo a >> a
+  $ hg commit -qAm aa
+  $ echo b >> b
+  $ hg commit -qAm bb
+  $ hg up ".^"
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ echo c >> c
+  $ hg commit -qAm cc
+  $ hg rebase
+  abort: you must specify a destination
+  (use: hg rebase -d REV)
+  [255]
+  $ hg rebase -d 1
+  rebasing 2:5db65b93a12b "cc" (tip)
+  saved backup bundle to $TESTTMP/repo/.hg/strip-backup/5db65b93a12b-4fb789ec-backup.hg (glob)