pitrou commented on a change in pull request #12411:
URL: https://github.com/apache/arrow/pull/12411#discussion_r809064478



##########
File path: dev/archery/archery/integration/tester_cpp.py
##########
@@ -23,39 +23,46 @@
 from .util import run_cmd, ARROW_ROOT_DEFAULT, log
 
 
+_EXE_PATH = os.environ.get(
+    "ARROW_CPP_EXE_PATH", os.path.join(ARROW_ROOT_DEFAULT, "cpp/build/debug")
+)
+_INTEGRATION_EXE = os.path.join(_EXE_PATH, "arrow-json-integration-test")
+_STREAM_TO_FILE = os.path.join(_EXE_PATH, "arrow-stream-to-file")
+_FILE_TO_STREAM = os.path.join(_EXE_PATH, "arrow-file-to-stream")
+
+_FLIGHT_SERVER_CMD = [os.path.join(
+    _EXE_PATH, "flight-test-integration-server")]
+_FLIGHT_CLIENT_CMD = [
+    os.path.join(_EXE_PATH, "flight-test-integration-client"),
+    "-host",
+    "localhost",
+]
+
+
 class CPPTester(Tester):
     PRODUCER = True
     CONSUMER = True
     FLIGHT_SERVER = True
     FLIGHT_CLIENT = True
 
-    EXE_PATH = os.environ.get(
-        'ARROW_CPP_EXE_PATH',
-        os.path.join(ARROW_ROOT_DEFAULT, 'cpp/build/debug'))
-
-    CPP_INTEGRATION_EXE = os.path.join(EXE_PATH, 'arrow-json-integration-test')
-    STREAM_TO_FILE = os.path.join(EXE_PATH, 'arrow-stream-to-file')
-    FILE_TO_STREAM = os.path.join(EXE_PATH, 'arrow-file-to-stream')
-
-    FLIGHT_SERVER_CMD = [
-        os.path.join(EXE_PATH, 'flight-test-integration-server')]
-    FLIGHT_CLIENT_CMD = [
-        os.path.join(EXE_PATH, 'flight-test-integration-client'),
-        "-host", "localhost"]
-
-    name = 'C++'
+    name = "C++"
 
-    def _run(self, arrow_path=None, json_path=None, command='VALIDATE',
-             quirks=None):
-        cmd = [self.CPP_INTEGRATION_EXE, '--integration']
+    def _run(
+        self,
+        arrow_path=None,
+        json_path=None,
+        command="VALIDATE",
+        quirks=None
+    ):

Review comment:
       Is this the product of running a style checker? It seems the outcome is 
a net negative (a lot of additional vertical space).

##########
File path: dev/archery/archery/integration/runner.py
##########
@@ -18,6 +18,7 @@
 from collections import namedtuple
 from concurrent.futures import ThreadPoolExecutor
 from functools import partial
+from typing import Callable, List

Review comment:
       Can you keep the imports alphabetically ordered, i.e. move `typing` 
after `traceback` below?

##########
File path: dev/archery/archery/integration/tester_cpp.py
##########
@@ -23,39 +23,46 @@
 from .util import run_cmd, ARROW_ROOT_DEFAULT, log
 
 
+_EXE_PATH = os.environ.get(
+    "ARROW_CPP_EXE_PATH", os.path.join(ARROW_ROOT_DEFAULT, "cpp/build/debug")
+)
+_INTEGRATION_EXE = os.path.join(_EXE_PATH, "arrow-json-integration-test")
+_STREAM_TO_FILE = os.path.join(_EXE_PATH, "arrow-stream-to-file")
+_FILE_TO_STREAM = os.path.join(_EXE_PATH, "arrow-file-to-stream")
+
+_FLIGHT_SERVER_CMD = [os.path.join(
+    _EXE_PATH, "flight-test-integration-server")]
+_FLIGHT_CLIENT_CMD = [
+    os.path.join(_EXE_PATH, "flight-test-integration-client"),
+    "-host",
+    "localhost",
+]
+
+
 class CPPTester(Tester):
     PRODUCER = True
     CONSUMER = True
     FLIGHT_SERVER = True
     FLIGHT_CLIENT = True
 
-    EXE_PATH = os.environ.get(
-        'ARROW_CPP_EXE_PATH',
-        os.path.join(ARROW_ROOT_DEFAULT, 'cpp/build/debug'))
-
-    CPP_INTEGRATION_EXE = os.path.join(EXE_PATH, 'arrow-json-integration-test')
-    STREAM_TO_FILE = os.path.join(EXE_PATH, 'arrow-stream-to-file')
-    FILE_TO_STREAM = os.path.join(EXE_PATH, 'arrow-file-to-stream')
-
-    FLIGHT_SERVER_CMD = [
-        os.path.join(EXE_PATH, 'flight-test-integration-server')]
-    FLIGHT_CLIENT_CMD = [
-        os.path.join(EXE_PATH, 'flight-test-integration-client'),
-        "-host", "localhost"]
-
-    name = 'C++'
+    name = "C++"
 
-    def _run(self, arrow_path=None, json_path=None, command='VALIDATE',
-             quirks=None):
-        cmd = [self.CPP_INTEGRATION_EXE, '--integration']
+    def _run(
+        self,
+        arrow_path=None,
+        json_path=None,
+        command="VALIDATE",
+        quirks=None
+    ):
+        cmd = [_INTEGRATION_EXE, "--integration"]
 
         if arrow_path is not None:
-            cmd.append('--arrow=' + arrow_path)
+            cmd.append("--arrow=" + arrow_path)

Review comment:
       Such gratuitous style changes will pollute the git history and make `git 
annotate` less useful. I don't know if it's easy for you to undo them?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to