[PATCH] D127864: [llvm-ar] Add object mode option -X for AIX

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/test/tools/llvm-ar/option-X.test:14
+
+## Test -X option for the create a new archive operation 
+# RUN: rm -f %t.a
----------------
jhenderson wrote:
> That being said, this first test case doesn't test the new option at all. It just uses the default behaviour.
it test the default option is as -X32 without -X option in command and no "OBJECT_MODE" set in environment.


================
Comment at: llvm/test/tools/llvm-ar/option-X.test:30
+# RUN: rm -f %t.a
+# RUN: export "OBJECT_MODE=64"
+# RUN: llvm-ar -q -c -X32 %t.a xcoff32.o elf32.o xcoff64.o elf64.o
----------------
jhenderson wrote:
> Add comments explaining what each test case is doing. E.g. here it is saying that the -X option overrides the environment variable.
> 
> Additionally, the norm for setting environment variables is to use the `env` command at the start of the RUN line you want to modify, e.g.
> ```
> # RUN: env OBJECT_MODE=64 llvm-ar ...
> ```
thanks


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127864/new/

https://reviews.llvm.org/D127864