Patchwork D6814: revlog: add a `sidedata` parameters to addrevision

login
register
mail settings
Submitter phabricator
Date Sept. 7, 2019, 9:29 a.m.
Message ID <differential-rev-PHID-DREV-dh7ikgl5wh4ukupr47zz-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/41518/
State New
Headers show

Comments

phabricator - Sept. 7, 2019, 9:29 a.m.
marmoute created this revision.
marmoute added reviewers: yuja, durin42.
Herald added a reviewer: indygreg.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  If we want to eventually store sidedata we need to be able to pass them along.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  hgext/remotefilelog/remotefilelog.py
  mercurial/revlog.py

CHANGE DETAILS




To: marmoute, yuja, durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - Sept. 7, 2019, 5:12 p.m.
indygreg added inline comments.

INLINE COMMENTS

> remotefilelog.py:133
>      def addrevision(self, text, transaction, linknode, p1, p2, cachedelta=None,
> -                    node=None, flags=revlog.REVIDX_DEFAULT_FLAGS):
> +                    node=None, flags=revlog.REVIDX_DEFAULT_FLAGS, sidedata=()):
>          # text passed to "addrevision" includes hg filelog metadata header

Why an empty tuple here? Isn't `None` more Pythonic?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6814/new/

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

To: marmoute, yuja, durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - Sept. 7, 2019, 5:15 p.m.
marmoute added inline comments.

INLINE COMMENTS

> indygreg wrote in remotefilelog.py:133
> Why an empty tuple here? Isn't `None` more Pythonic?

because it can be iterated over as a noop.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6814/new/

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

To: marmoute, yuja, durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - Sept. 9, 2019, 5:52 p.m.
This revision now requires changes to proceed.
durin42 added inline comments.
durin42 requested changes to this revision.

INLINE COMMENTS

> marmoute wrote in remotefilelog.py:133
> because it can be iterated over as a noop.

I agree with Greg: if your intent is "optional iterable of data", the tuple is misguided and will cause problems for typecheckers. I know it adds two lines, but None is the correct way to spell "optional value that was empty" in this case.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6814/new/

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

To: marmoute, yuja, durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - Sept. 9, 2019, 5:56 p.m.
marmoute added a comment.


  Okay, I'll update it.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6814/new/

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

To: marmoute, yuja, durin42, indygreg, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1813,7 +1813,8 @@ 
         """
 
     def addrevision(self, text, transaction, link, p1, p2, cachedelta=None,
-                    node=None, flags=REVIDX_DEFAULT_FLAGS, deltacomputer=None):
+                    node=None, flags=REVIDX_DEFAULT_FLAGS, deltacomputer=None,
+                    sidedata=()):
         """add a revision to the log
 
         text - the revision data to add
diff --git a/hgext/remotefilelog/remotefilelog.py b/hgext/remotefilelog/remotefilelog.py
--- a/hgext/remotefilelog/remotefilelog.py
+++ b/hgext/remotefilelog/remotefilelog.py
@@ -130,7 +130,7 @@ 
         return data
 
     def addrevision(self, text, transaction, linknode, p1, p2, cachedelta=None,
-                    node=None, flags=revlog.REVIDX_DEFAULT_FLAGS):
+                    node=None, flags=revlog.REVIDX_DEFAULT_FLAGS, sidedata=()):
         # text passed to "addrevision" includes hg filelog metadata header
         if node is None:
             node = storageutil.hashrevisionsha1(text, p1, p2)