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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
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

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)



To: marmoute, yuja, durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
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
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
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
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
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
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
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
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
marmoute updated this revision to Diff 16480.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D6814?vs=16419&id=16480

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

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

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

CHANGE DETAILS

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1817,7 +1817,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=None):
         """add a revision to the log
 
         text - the revision data to add
@@ -1836,6 +1837,9 @@
             raise error.RevlogError(_("attempted to add linkrev -1 to %s")
                                     % self.indexfile)
 
+        if sidedata is None:
+            sidedata = {}
+
         if flags:
             node = node or self.hash(text, p1, p2)
 
diff --git a/hgext/remotefilelog/remotefilelog.py b/hgext/remotefilelog/remotefilelog.py
--- a/hgext/remotefilelog/remotefilelog.py
+++ b/hgext/remotefilelog/remotefilelog.py
@@ -129,7 +129,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)



To: marmoute, yuja, durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
marmoute updated this revision to Diff 16494.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D6814?vs=16480&id=16494

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

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

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

CHANGE DETAILS

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1816,7 +1816,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=None):
         """add a revision to the log
 
         text - the revision data to add
@@ -1835,6 +1836,9 @@
             raise error.RevlogError(_("attempted to add linkrev -1 to %s")
                                     % self.indexfile)
 
+        if sidedata is None:
+            sidedata = {}
+
         if flags:
             node = node or self.hash(text, p1, p2)
 
diff --git a/hgext/remotefilelog/remotefilelog.py b/hgext/remotefilelog/remotefilelog.py
--- a/hgext/remotefilelog/remotefilelog.py
+++ b/hgext/remotefilelog/remotefilelog.py
@@ -129,7 +129,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)



To: marmoute, yuja, durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
This revision now requires changes to proceed.
durin42 added inline comments.
durin42 requested changes to this revision.

INLINE COMMENTS

> remotefilelog.py:132
>      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

You fixed the default in revlog.py but not in remotefilelog.py.

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
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
marmoute updated this revision to Diff 16600.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D6814?vs=16494&id=16600

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

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

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

CHANGE DETAILS

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1816,7 +1816,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=None):
         """add a revision to the log
 
         text - the revision data to add
@@ -1835,6 +1836,9 @@
             raise error.RevlogError(_("attempted to add linkrev -1 to %s")
                                     % self.indexfile)
 
+        if sidedata is None:
+            sidedata = {}
+
         if flags:
             node = node or self.hash(text, p1, p2)
 
diff --git a/hgext/remotefilelog/remotefilelog.py b/hgext/remotefilelog/remotefilelog.py
--- a/hgext/remotefilelog/remotefilelog.py
+++ b/hgext/remotefilelog/remotefilelog.py
@@ -129,10 +129,12 @@
         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=None):
         # text passed to "addrevision" includes hg filelog metadata header
         if node is None:
             node = storageutil.hashrevisionsha1(text, p1, p2)
+        if sidedata is None:
+            sidedata = {}
 
         meta, metaoffset = storageutil.parsemeta(text)
         rawtext, validatehash = self._processflagswrite(text, flags)



To: marmoute, yuja, durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
marmoute marked an inline comment as done.
marmoute updated this revision to Diff 16601.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D6814?vs=16600&id=16601

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

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

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

CHANGE DETAILS

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1816,7 +1816,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=None):
         """add a revision to the log
 
         text - the revision data to add
@@ -1835,6 +1836,9 @@
             raise error.RevlogError(_("attempted to add linkrev -1 to %s")
                                     % self.indexfile)
 
+        if sidedata is None:
+            sidedata = {}
+
         if flags:
             node = node or self.hash(text, p1, p2)
 
diff --git a/hgext/remotefilelog/remotefilelog.py b/hgext/remotefilelog/remotefilelog.py
--- a/hgext/remotefilelog/remotefilelog.py
+++ b/hgext/remotefilelog/remotefilelog.py
@@ -129,10 +129,13 @@
         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=None):
         # text passed to "addrevision" includes hg filelog metadata header
         if node is None:
             node = storageutil.hashrevisionsha1(text, p1, p2)
+        if sidedata is None:
+            sidedata = {}
 
         meta, metaoffset = storageutil.parsemeta(text)
         rawtext, validatehash = self._processflagswrite(text, flags)



To: marmoute, yuja, durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
Closed by commit rHG33532939c667: revlog: add a `sidedata` parameters to addrevision (authored by marmoute).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Review".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D6814?vs=16601&id=16634

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

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

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

CHANGE DETAILS

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1816,7 +1816,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=None):
         """add a revision to the log
 
         text - the revision data to add
@@ -1835,6 +1836,9 @@
             raise error.RevlogError(_("attempted to add linkrev -1 to %s")
                                     % self.indexfile)
 
+        if sidedata is None:
+            sidedata = {}
+
         if flags:
             node = node or self.hash(text, p1, p2)
 
diff --git a/hgext/remotefilelog/remotefilelog.py b/hgext/remotefilelog/remotefilelog.py
--- a/hgext/remotefilelog/remotefilelog.py
+++ b/hgext/remotefilelog/remotefilelog.py
@@ -129,10 +129,13 @@
         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=None):
         # text passed to "addrevision" includes hg filelog metadata header
         if node is None:
             node = storageutil.hashrevisionsha1(text, p1, p2)
+        if sidedata is None:
+            sidedata = {}
 
         meta, metaoffset = storageutil.parsemeta(text)
         rawtext, validatehash = self._processflagswrite(text, flags)



To: marmoute, yuja, durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel