Submitter | phabricator |
---|---|
Date | Aug. 17, 2019, 11:45 p.m. |
Message ID | <differential-rev-PHID-DREV-vfcfyiuel5ffp37hlxs6-req@mercurial-scm.org> |
Download | mbox | patch |
Permalink | /patch/41347/ |
State | Superseded |
Headers | show |
Comments
martinvonz added a comment. > I was trying to understand current interfaces and write new ones and I realized > we need to improve how current interfaces are organised. And what was the reason we need to improve it? I assume we don't really "need" to change it. Will it somehow help with future patches? Or you just like this structure better? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6741/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6741 To: pulkit, indygreg, durin42, martinvonz, #hg-reviewers Cc: mercurial-devel
pulkit added a comment. In D6741#98947 <https://phab.mercurial-scm.org/D6741#98947>, @martinvonz wrote: >> I was trying to understand current interfaces and write new ones and I realized >> we need to improve how current interfaces are organised. > > And what was the reason we need to improve it? I assume we don't really "need" to change it. Will it somehow help with future patches? Or you just like this structure better? Looking through Augie's hgit patch, I found we need to add more interfaces and decided to work on adding for `store.basicstore`. I found all the current interfaces in repository.py which has grown very large. I decided to create a new file to have interface for the store class, and maybe a new one for dirstate too, and having them in a separate folder dedicated to interfaces will be nice. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6741/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6741 To: pulkit, indygreg, durin42, martinvonz, #hg-reviewers Cc: mercurial-devel
durin42 added a comment. In D6741#99023 <https://phab.mercurial-scm.org/D6741#99023>, @pulkit wrote: > In D6741#98947 <https://phab.mercurial-scm.org/D6741#98947>, @martinvonz wrote: > >>> I was trying to understand current interfaces and write new ones and I realized >>> we need to improve how current interfaces are organised. >> >> And what was the reason we need to improve it? I assume we don't really "need" to change it. Will it somehow help with future patches? Or you just like this structure better? > > Looking through Augie's hgit patch, I found we need to add more interfaces and decided to work on adding for `store.basicstore`. I found all the current interfaces in repository.py which has grown very large. I decided to create a new file to have interface for the store class, and maybe a new one for dirstate too, and having them in a separate folder dedicated to interfaces will be nice. I'm inclined to agree - the repository.py file full of interfaces has gotten crufty. It'd be nice to split it out into more files by topic. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6741/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6741 To: pulkit, indygreg, durin42, martinvonz, #hg-reviewers Cc: mercurial-devel
martinvonz added a comment. In D6741#99030 <https://phab.mercurial-scm.org/D6741#99030>, @durin42 wrote: > In D6741#99023 <https://phab.mercurial-scm.org/D6741#99023>, @pulkit wrote: > >> In D6741#98947 <https://phab.mercurial-scm.org/D6741#98947>, @martinvonz wrote: >> >>>> I was trying to understand current interfaces and write new ones and I realized >>>> we need to improve how current interfaces are organised. >>> >>> And what was the reason we need to improve it? I assume we don't really "need" to change it. Will it somehow help with future patches? Or you just like this structure better? >> >> Looking through Augie's hgit patch, I found we need to add more interfaces and decided to work on adding for `store.basicstore`. I found all the current interfaces in repository.py which has grown very large. I decided to create a new file to have interface for the store class, and maybe a new one for dirstate too, and having them in a separate folder dedicated to interfaces will be nice. > > I'm inclined to agree - the repository.py file full of interfaces has gotten crufty. It'd be nice to split it out into more files by topic. That makes sense to me too. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6741/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6741 To: pulkit, indygreg, durin42, martinvonz, #hg-reviewers Cc: mercurial-devel
This revision is now accepted and ready to land. indygreg added a comment. indygreg accepted this revision. It sounds like we're in agreement that interfaces need to be split. Moving everything to a sub-package seems like a logical first step. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6741/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6741 To: pulkit, indygreg, durin42, martinvonz, #hg-reviewers Cc: mercurial-devel
Patch
diff --git a/tests/wireprotosimplecache.py b/tests/wireprotosimplecache.py --- a/tests/wireprotosimplecache.py +++ b/tests/wireprotosimplecache.py @@ -10,12 +10,14 @@ from mercurial import ( extensions, registrar, - repository, util, wireprotoserver, wireprototypes, wireprotov2server, ) +from mercurial.interfaces import ( + repository, +) from mercurial.utils import ( interfaceutil, stringutil, diff --git a/tests/test-check-interfaces.py b/tests/test-check-interfaces.py --- a/tests/test-check-interfaces.py +++ b/tests/test-check-interfaces.py @@ -14,6 +14,9 @@ 'test-repo']): sys.exit(80) +from mercurial.interfaces import ( + repository, +) from mercurial.thirdparty.zope import ( interface as zi, ) @@ -27,7 +30,6 @@ localrepo, manifest, pycompat, - repository, revlog, sshpeer, statichttprepo, diff --git a/tests/simplestorerepo.py b/tests/simplestorerepo.py --- a/tests/simplestorerepo.py +++ b/tests/simplestorerepo.py @@ -32,11 +32,13 @@ localrepo, mdiff, pycompat, - repository, revlog, store, verify, ) +from mercurial.interfaces import ( + repository, +) from mercurial.utils import ( cborutil, interfaceutil, diff --git a/tests/pullext.py b/tests/pullext.py --- a/tests/pullext.py +++ b/tests/pullext.py @@ -13,6 +13,8 @@ error, extensions, localrepo, +) +from mercurial.interfaces import ( repository, ) diff --git a/tests/notcapable b/tests/notcapable --- a/tests/notcapable +++ b/tests/notcapable @@ -6,7 +6,8 @@ fi cat > notcapable-$CAP.py << EOF -from mercurial import extensions, localrepo, repository +from mercurial import extensions, localrepo +from mercurial.interfaces import repository def extsetup(ui): extensions.wrapfunction(repository.peer, 'capable', wrapcapable) extensions.wrapfunction(localrepo.localrepository, 'peer', wrappeer) diff --git a/setup.py b/setup.py --- a/setup.py +++ b/setup.py @@ -1067,6 +1067,7 @@ 'mercurial.cext', 'mercurial.cffi', 'mercurial.hgweb', + 'mercurial.interfaces', 'mercurial.pure', 'mercurial.thirdparty', 'mercurial.thirdparty.attr', diff --git a/mercurial/wireprotov1peer.py b/mercurial/wireprotov1peer.py --- a/mercurial/wireprotov1peer.py +++ b/mercurial/wireprotov1peer.py @@ -22,10 +22,12 @@ error, pushkey as pushkeymod, pycompat, - repository, util, wireprototypes, ) +from .interfaces import ( + repository, +) from .utils import ( interfaceutil, ) diff --git a/mercurial/utils/storageutil.py b/mercurial/utils/storageutil.py --- a/mercurial/utils/storageutil.py +++ b/mercurial/utils/storageutil.py @@ -22,8 +22,8 @@ error, mdiff, pycompat, - repository, ) +from ..interfaces import repository _nullhash = hashlib.sha1(nullid) diff --git a/mercurial/testing/storage.py b/mercurial/testing/storage.py --- a/mercurial/testing/storage.py +++ b/mercurial/testing/storage.py @@ -17,6 +17,8 @@ from .. import ( error, mdiff, +) +from ..interfaces import ( repository, ) from ..utils import ( diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py --- a/mercurial/streamclone.py +++ b/mercurial/streamclone.py @@ -12,13 +12,15 @@ import struct from .i18n import _ +from .interfaces import ( + repository, +) from . import ( cacheutil, error, narrowspec, phases, pycompat, - repository, store, util, ) diff --git a/mercurial/revlogutils/constants.py b/mercurial/revlogutils/constants.py --- a/mercurial/revlogutils/constants.py +++ b/mercurial/revlogutils/constants.py @@ -9,7 +9,7 @@ from __future__ import absolute_import -from .. import ( +from ..interfaces import ( repository, ) diff --git a/mercurial/revlog.py b/mercurial/revlog.py --- a/mercurial/revlog.py +++ b/mercurial/revlog.py @@ -65,10 +65,12 @@ mdiff, policy, pycompat, - repository, templatefilters, util, ) +from .interfaces import ( + repository, +) from .revlogutils import ( deltas as deltautil, flagutil, diff --git a/mercurial/narrowspec.py b/mercurial/narrowspec.py --- a/mercurial/narrowspec.py +++ b/mercurial/narrowspec.py @@ -8,11 +8,13 @@ from __future__ import absolute_import from .i18n import _ +from .interfaces import ( + repository, +) from . import ( error, match as matchmod, merge, - repository, scmutil, sparse, util, diff --git a/mercurial/manifest.py b/mercurial/manifest.py --- a/mercurial/manifest.py +++ b/mercurial/manifest.py @@ -24,10 +24,12 @@ mdiff, policy, pycompat, - repository, revlog, util, ) +from .interfaces import ( + repository, +) from .utils import ( interfaceutil, ) diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -52,7 +52,6 @@ phases, pushkey, pycompat, - repository, repoview, revset, revsetlang, @@ -66,6 +65,11 @@ util, vfs as vfsmod, ) + +from .interfaces import ( + repository, +) + from .utils import ( interfaceutil, procutil, diff --git a/mercurial/repository.py b/mercurial/interfaces/repository.py rename from mercurial/repository.py rename to mercurial/interfaces/repository.py --- a/mercurial/repository.py +++ b/mercurial/interfaces/repository.py @@ -7,11 +7,11 @@ from __future__ import absolute_import -from .i18n import _ -from . import ( +from ..i18n import _ +from .. import ( error, ) -from .utils import ( +from ..utils import ( interfaceutil, ) diff --git a/mercurial/interfaces/__init__.py b/mercurial/interfaces/__init__.py new file mode 100644 diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py --- a/mercurial/httppeer.py +++ b/mercurial/httppeer.py @@ -16,12 +16,14 @@ import weakref from .i18n import _ +from .interfaces import ( + repository, +) from . import ( bundle2, error, httpconnection, pycompat, - repository, statichttprepo, url as urlmod, util, diff --git a/mercurial/hg.py b/mercurial/hg.py --- a/mercurial/hg.py +++ b/mercurial/hg.py @@ -39,7 +39,6 @@ node, phases, pycompat, - repository as repositorymod, scmutil, sshpeer, statichttprepo, @@ -51,6 +50,10 @@ vfs as vfsmod, ) +from .interfaces import ( + repository as repositorymod, +) + release = lock.release # shared features diff --git a/mercurial/filelog.py b/mercurial/filelog.py --- a/mercurial/filelog.py +++ b/mercurial/filelog.py @@ -14,8 +14,10 @@ ) from . import ( error, + revlog, +) +from .interfaces import ( repository, - revlog, ) from .utils import ( interfaceutil, diff --git a/mercurial/exchangev2.py b/mercurial/exchangev2.py --- a/mercurial/exchangev2.py +++ b/mercurial/exchangev2.py @@ -22,8 +22,10 @@ narrowspec, phases, pycompat, + setdiscovery, +) +from .interfaces import ( repository, - setdiscovery, ) def pull(pullop): diff --git a/mercurial/exchange.py b/mercurial/exchange.py --- a/mercurial/exchange.py +++ b/mercurial/exchange.py @@ -34,7 +34,6 @@ phases, pushkey, pycompat, - repository, scmutil, sslutil, streamclone, @@ -42,6 +41,9 @@ util, wireprototypes, ) +from .interfaces import ( + repository, +) from .utils import ( stringutil, ) diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py --- a/mercurial/changegroup.py +++ b/mercurial/changegroup.py @@ -25,8 +25,11 @@ mdiff, phases, pycompat, + util, +) + +from .interfaces import ( repository, - util, ) _CHANGEGROUPV1_DELTA_HEADER = struct.Struct("20s20s20s20s") diff --git a/hgext/sqlitestore.py b/hgext/sqlitestore.py --- a/hgext/sqlitestore.py +++ b/hgext/sqlitestore.py @@ -70,10 +70,12 @@ mdiff, pycompat, registrar, - repository, util, verify, ) +from mercurial.interfaces import ( + repository, +) from mercurial.utils import ( interfaceutil, storageutil, diff --git a/hgext/narrow/narrowcommands.py b/hgext/narrow/narrowcommands.py --- a/hgext/narrow/narrowcommands.py +++ b/hgext/narrow/narrowcommands.py @@ -25,12 +25,14 @@ pycompat, registrar, repair, - repository, repoview, sparse, util, wireprototypes, ) +from mercurial.interfaces import ( + repository, +) table = {} command = registrar.command(table) diff --git a/hgext/narrow/narrowbundle2.py b/hgext/narrow/narrowbundle2.py --- a/hgext/narrow/narrowbundle2.py +++ b/hgext/narrow/narrowbundle2.py @@ -23,10 +23,12 @@ localrepo, narrowspec, repair, - repository, util, wireprototypes, ) +from mercurial.interfaces import ( + repository, +) from mercurial.utils import ( stringutil, ) diff --git a/hgext/narrow/__init__.py b/hgext/narrow/__init__.py --- a/hgext/narrow/__init__.py +++ b/hgext/narrow/__init__.py @@ -17,6 +17,9 @@ from mercurial import ( localrepo, registrar, +) + +from mercurial.interfaces import ( repository, ) diff --git a/hgext/lfs/wrapper.py b/hgext/lfs/wrapper.py --- a/hgext/lfs/wrapper.py +++ b/hgext/lfs/wrapper.py @@ -21,7 +21,6 @@ exchange, exthelper, localrepo, - repository, revlog, scmutil, upgrade, @@ -30,6 +29,10 @@ wireprotov1server, ) +from mercurial.interfaces import ( + repository, +) + from mercurial.utils import ( storageutil, stringutil, diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py --- a/hgext/lfs/__init__.py +++ b/hgext/lfs/__init__.py @@ -141,13 +141,16 @@ minifileset, node, pycompat, - repository, revlog, scmutil, templateutil, util, ) +from mercurial.interfaces import ( + repository, +) + from . import ( blobstore, wireprotolfsserver, diff --git a/contrib/import-checker.py b/contrib/import-checker.py --- a/contrib/import-checker.py +++ b/contrib/import-checker.py @@ -28,6 +28,7 @@ 'mercurial.hgweb.common', 'mercurial.hgweb.request', 'mercurial.i18n', + 'mercurial.interfaces', 'mercurial.node', # for revlog to re-export constant to extensions 'mercurial.revlogutils.constants',