On 3/30/17 12:57 AM, Jun Wu wrote:
# HG changeset patch
# User Jun Wu <qu...@fb.com>
# Date 1490831529 25200
#      Wed Mar 29 16:52:09 2017 -0700
# Node ID 5ca313b3da12d145f1d49a85dd8b753e22d51521
# Parent  265ea657d75905fb59a27194a75aaff49be94598
perf: workaround check-code

The code int his series all looks good to me. I'll mark as pre-reviewed in patchwork. The only question I have is whether we prefer to leave check-code errors in place inside the test, or workaround it with hacks like this?

I'd prefer to leave the "expected failures" inside the test because it documents that we intend to keep them (probably with a comment at the site of the "error"). Also, if the check-code regex gets smarter in the future, we don't have to go and change our hack again.

However, I don't know what the rest of the community thinks about this. I don't have a strong enough opinion, so I guess I'm -0 on this patch, but +1 on the rest of the series. I'll be happy as long as the first four get in regardless of if patch 5 is included or not.

The check-code suggests using rev instead of node for revlog.revision. But
early Mercurial does not support that (see 9117c6561b0b). So let's just
workaround check-code in perf.py

diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -847,4 +847,5 @@ def perfrevlog(ui, repo, file_=None, sta
      def d():
          r = cmdutil.openrevlog(repo, 'perfrevlog', file_, opts)
+        r2 = r # workaround check-code
startrev = 0
@@ -857,5 +858,5 @@ def perfrevlog(ui, repo, file_=None, sta
for x in xrange(startrev, endrev, dist):
-            r.revision(r.node(x))
+            r2.revision(r.node(x))
timer(d)
diff --git a/tests/test-check-code.t b/tests/test-check-code.t
--- a/tests/test-check-code.t
+++ b/tests/test-check-code.t
@@ -10,7 +10,4 @@ New errors are not allowed. Warnings are
    $ hg locate -X contrib/python-zstandard -X hgext/fsmonitor/pywatchman |
    > sed 's-\\-/-g' | xargs "$check_code" --warnings --per-file=0 || false
-  contrib/perf.py:859:
-   >             r.revision(r.node(x))
-   don't covert rev to node before passing to revision(nodeorrev)
    Skipping i18n/polib.py it has no-che?k-code (glob)
    mercurial/demandimport.py:312:


_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to