Patchwork D1070: config: graduate experimental.updatecheck to commands.update.check

login
register
mail settings
Submitter phabricator
Date Oct. 14, 2017, 7:15 a.m.
Message ID <differential-rev-PHID-DREV-t6l365fuefnringksqdf-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/24885/
State Superseded
Headers show

Comments

phabricator - Oct. 14, 2017, 7:15 a.m.
durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  No backwards compatibility provided for the legacy name. Perhaps we
  should provide that for a release cycle? I don't feel strongly.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1070

AFFECTED FILES
  mercurial/configitems.py
  mercurial/hg.py
  tests/test-pull-update.t
  tests/test-update-branches.t

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - Oct. 14, 2017, 7:53 a.m.
morisgi added inline comments.

INLINE COMMENTS

> configitems.py:182
>  )
> +coreconfigitem('commands', 'update.check',
> +    default=None,

Should it be documented in the help then?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1070

To: durin42, #hg-reviewers
Cc: morisgi, mercurial-devel
phabricator - Oct. 14, 2017, 3:45 p.m.
lothiraldan requested changes to this revision.
lothiraldan added a comment.
This revision now requires changes to proceed.


  Could you add an alias to `experimental.updatecheck` so old configurations will not break. I'm not saying to keep it forever but at least have it when we change it.
  
  +1 on documenting the option now that it's not experimental anymore

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1070

To: durin42, #hg-reviewers, lothiraldan
Cc: lothiraldan, morisgi, mercurial-devel
phabricator - Oct. 14, 2017, 8:40 p.m.
durin42 marked an inline comment as done.
durin42 added a comment.


  Added some docs.
  
  In https://phab.mercurial-scm.org/D1070#17994, @lothiraldan wrote:
  
  > Could you add an alias to `experimental.updatecheck` so old configurations will not break. I'm not saying to keep it forever but at least have it when we change it.
  >
  > +1 on documenting the option now that it's not experimental anymore
  
  
  Done.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1070

To: durin42, #hg-reviewers, lothiraldan
Cc: lothiraldan, morisgi, mercurial-devel
phabricator - Oct. 14, 2017, 8:49 p.m.
indygreg accepted this revision.
indygreg added a comment.


  I'm not crazy about the code style. But since this code only lives for a release, I'm OK taking it. I'll accept this but will defer to someone else to queue it in case there are strong opinions about the style.

INLINE COMMENTS

> hg.py:761-766
>          if updatecheck not in ('abort', 'none', 'linear', 'noconflict'):
> -            # If not configured, or invalid value configured
> -            updatecheck = 'linear'
> +            # pre-4.4 compat on the old spelling of this config item
> +            updatecheck = ui.config('experimental', 'updatecheck')
> +            if updatecheck not in ('abort', 'none', 'linear', 'noconflict'):
> +                # If not configured, or invalid value configured
> +                updatecheck = 'linear'

How do you feel about this alternative approach?

  updatecheck = ui.config('commands', 'update.check')
  if updatecheck is None:
      updatecheck = ui.config('experimental', 'updatecheck')
  if updatecheck not in ('abort', ...):
      updatecheck = 'linear'

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1070

To: durin42, #hg-reviewers, lothiraldan, indygreg
Cc: indygreg, lothiraldan, morisgi, mercurial-devel
phabricator - Oct. 14, 2017, 8:50 p.m.
durin42 added inline comments.

INLINE COMMENTS

> indygreg wrote in hg.py:761-766
> How do you feel about this alternative approach?
> 
>   updatecheck = ui.config('commands', 'update.check')
>   if updatecheck is None:
>       updatecheck = ui.config('experimental', 'updatecheck')
>   if updatecheck not in ('abort', ...):
>       updatecheck = 'linear'

Sure, that seems strictly better. Done.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1070

To: durin42, #hg-reviewers, lothiraldan, indygreg
Cc: indygreg, lothiraldan, morisgi, mercurial-devel
phabricator - Oct. 16, 2017, 8:57 a.m.
lothiraldan added a comment.


  Sorry, I was not clear, I was thinking about adding an alias in the config registration, like that:
  
    coreconfigitem('commands', 'update.check',
        default=None,
        alias=[('experimental', 'updatecheck')]
    )
  
  Should I send a follow-up?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1070

To: durin42, #hg-reviewers, lothiraldan, indygreg
Cc: indygreg, lothiraldan, morisgi, mercurial-devel
phabricator - Oct. 16, 2017, 1:50 p.m.
durin42 added a comment.


  Ugh, please send a follow-up, sorry about that.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1070

To: durin42, #hg-reviewers, lothiraldan, indygreg
Cc: indygreg, lothiraldan, morisgi, mercurial-devel

Patch

diff --git a/tests/test-update-branches.t b/tests/test-update-branches.t
--- a/tests/test-update-branches.t
+++ b/tests/test-update-branches.t
@@ -198,8 +198,8 @@ 
   parent=1
   M foo
 
-  $ echo '[experimental]' >> .hg/hgrc
-  $ echo 'updatecheck = abort' >> .hg/hgrc
+  $ echo '[commands]' >> .hg/hgrc
+  $ echo 'update.check = abort' >> .hg/hgrc
 
   $ revtest 'none dirty linear' dirty 1 2
   abort: uncommitted changes
@@ -215,7 +215,7 @@ 
   2 files updated, 0 files merged, 0 files removed, 0 files unresolved
   parent=2
 
-  $ echo 'updatecheck = none' >> .hg/hgrc
+  $ echo 'update.check = none' >> .hg/hgrc
 
   $ revtest 'none dirty cross'  dirty 3 4
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
@@ -258,7 +258,7 @@ 
   >>>>>>> destination:  d047485b3896 b1 - test: 4
   $ rm a.orig
 
-  $ echo 'updatecheck = noconflict' >> .hg/hgrc
+  $ echo 'update.check = noconflict' >> .hg/hgrc
 
   $ revtest 'none dirty cross'  dirty 3 4
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
@@ -315,7 +315,7 @@ 
   $ hg up -q 4
 
 Uses default value of "linear" when value is misspelled
-  $ echo 'updatecheck = linyar' >> .hg/hgrc
+  $ echo 'update.check = linyar' >> .hg/hgrc
 
   $ revtest 'dirty cross'  dirty 3 4
   abort: uncommitted changes
diff --git a/tests/test-pull-update.t b/tests/test-pull-update.t
--- a/tests/test-pull-update.t
+++ b/tests/test-pull-update.t
@@ -19,7 +19,7 @@ 
 Should respect config to disable dirty update
   $ hg co -qC 0
   $ echo 2 > foo
-  $ hg --config experimental.updatecheck=abort pull -u ../tt
+  $ hg --config commands.update.check=abort pull -u ../tt
   pulling from ../tt
   searching for changes
   adding changesets
diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -757,7 +757,7 @@ 
     This returns whether conflict is detected at updating or not.
     """
     if updatecheck is None:
-        updatecheck = ui.config('experimental', 'updatecheck')
+        updatecheck = ui.config('commands', 'update.check')
         if updatecheck not in ('abort', 'none', 'linear', 'noconflict'):
             # If not configured, or invalid value configured
             updatecheck = 'linear'
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -179,6 +179,9 @@ 
 coreconfigitem('commands', 'status.verbose',
     default=False,
 )
+coreconfigitem('commands', 'update.check',
+    default=None,
+)
 coreconfigitem('commands', 'update.requiredest',
     default=False,
 )
@@ -409,9 +412,6 @@ 
 coreconfigitem('experimental', 'treemanifest',
     default=False,
 )
-coreconfigitem('experimental', 'updatecheck',
-    default=None,
-)
 coreconfigitem('extensions', '.*',
     default=None,
     generic=True,