kou commented on code in PR #46530:
URL: https://github.com/apache/arrow/pull/46530#discussion_r2110634441


##########
dev/archery/archery/cli.py:
##########
@@ -844,29 +868,28 @@ def integration(with_all=False, random_seed=12345, 
**args):
     implementations = ['cpp', 'csharp', 'java', 'js', 'go', 'nanoarrow', 
'rust']
     formats = ['ipc', 'flight', 'c_data']
 
-    enabled_implementations = 0
+    enabled_implementations : bool = False
     for lang in implementations:
         param = f'with_{lang}'
         if with_all:
             args[param] = with_all
-        enabled_implementations += args[param]
+        enabled_implementations = True
 
-    enabled_formats = 0
+    enabled_formats : bool = False
     for fmt in formats:
-        param = f'run_{fmt}'
-        enabled_formats += args[param]
+        enabled_formats = True
 
     if gen_path:
         # XXX See GH-37575: this option is only used by the JS test suite
         # and might not be useful anymore.
         os.makedirs(gen_path, exist_ok=True)
         write_js_test_json(gen_path)
     else:
-        if enabled_formats == 0:
+        if enabled_formats == False:
             raise click.UsageError(
                 "Need to enable at least one format to test "
                 "(IPC, Flight, C Data Interface); try --help")
-        if enabled_implementations == 0:
+        if enabled_implementations == False:

Review Comment:
   ```suggestion
           if not enabled_implementations:
   ```



##########
dev/archery/archery/cli.py:
##########
@@ -745,6 +745,30 @@ def _set_default(opt, default):
 @click.option('--with-rust', type=bool, default=False,
               help='Include Rust in integration tests',
               envvar="ARCHERY_INTEGRATION_WITH_RUST")
+@click.option('--with-external-library', type=click.Path(exists=True, 
file_okay=False, dir_okay=True, executable=False),
+              help='Include external library in integration tests',
+              envvar="ARCHERY_INTEGRATION_WITH_EXTERNAL_LIBRARY")
+@click.option('--external-library-IPC-producer', type=bool, default=False,
+              help='Set external library as supporting producing IPC in 
integration tests',
+              envvar="ARCHERY_INTEGRATION_EXTERNAL_LIBRARY_IPC_PRODUCER")
+@click.option('--external-library-IPC-consumer', type=bool, default=False,

Review Comment:
   ```suggestion
   @click.option('--external-library-ipc-consumer', type=bool, default=False,
   ```



##########
dev/archery/archery/cli.py:
##########
@@ -745,6 +745,30 @@ def _set_default(opt, default):
 @click.option('--with-rust', type=bool, default=False,
               help='Include Rust in integration tests',
               envvar="ARCHERY_INTEGRATION_WITH_RUST")
+@click.option('--with-external-library', type=click.Path(exists=True, 
file_okay=False, dir_okay=True, executable=False),
+              help='Include external library in integration tests',
+              envvar="ARCHERY_INTEGRATION_WITH_EXTERNAL_LIBRARY")
+@click.option('--external-library-IPC-producer', type=bool, default=False,

Review Comment:
   ```suggestion
   @click.option('--external-library-ipc-producer', type=bool, default=False,
   ```



##########
dev/archery/archery/cli.py:
##########
@@ -844,29 +868,28 @@ def integration(with_all=False, random_seed=12345, 
**args):
     implementations = ['cpp', 'csharp', 'java', 'js', 'go', 'nanoarrow', 
'rust']
     formats = ['ipc', 'flight', 'c_data']
 
-    enabled_implementations = 0
+    enabled_implementations : bool = False
     for lang in implementations:
         param = f'with_{lang}'
         if with_all:
             args[param] = with_all
-        enabled_implementations += args[param]
+        enabled_implementations = True

Review Comment:
   Do we need this...?
   
   ```suggestion
           if args[param]:
               enabled_implementations = True
   ```



##########
dev/archery/archery/cli.py:
##########
@@ -844,29 +868,28 @@ def integration(with_all=False, random_seed=12345, 
**args):
     implementations = ['cpp', 'csharp', 'java', 'js', 'go', 'nanoarrow', 
'rust']
     formats = ['ipc', 'flight', 'c_data']
 
-    enabled_implementations = 0
+    enabled_implementations : bool = False

Review Comment:
   If we want to use `bool` for this, we may need to rename this something like 
`have_enabled_implementation`.



##########
dev/archery/archery/cli.py:
##########
@@ -844,29 +868,28 @@ def integration(with_all=False, random_seed=12345, 
**args):
     implementations = ['cpp', 'csharp', 'java', 'js', 'go', 'nanoarrow', 
'rust']
     formats = ['ipc', 'flight', 'c_data']
 
-    enabled_implementations = 0
+    enabled_implementations : bool = False
     for lang in implementations:
         param = f'with_{lang}'
         if with_all:
             args[param] = with_all
-        enabled_implementations += args[param]
+        enabled_implementations = True
 
-    enabled_formats = 0
+    enabled_formats : bool = False
     for fmt in formats:
-        param = f'run_{fmt}'
-        enabled_formats += args[param]
+        enabled_formats = True
 
     if gen_path:
         # XXX See GH-37575: this option is only used by the JS test suite
         # and might not be useful anymore.
         os.makedirs(gen_path, exist_ok=True)
         write_js_test_json(gen_path)
     else:
-        if enabled_formats == 0:
+        if enabled_formats == False:

Review Comment:
   ```suggestion
           if not enabled_formats:
   ```



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to