[PATCH] D129311: [clang-format] Update return code

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.


================
Comment at: clang/tools/clang-format/git-clang-format:579
     with temporary_index_file(old_tree):
-      subprocess.check_call(['git', 'checkout', '--patch', new_tree])
+      subprocess.run(['git', 'checkout', '--patch', new_tree], check=True)
     index_tree = old_tree
----------------
Good catch with `check=True`. Should we add it to the other two instances of `run()` above?


================
Comment at: clang/tools/clang-format/git-clang-format:539-540
   # filter.
-  subprocess.check_call(['git', 'diff', '--diff-filter=M', old_tree, new_tree,
-                         '--'])
+  subprocess.check_call(['git', 'diff', '--diff-filter=M',
+                        old_tree, new_tree, '--exit-code', '--'])
 
----------------
sridhar_gopinath wrote:
> owenpan wrote:
> > `--exit-code` is implied?
> `--exit-code` is not implied. From the docs:
> ```
> --exit-code
> Make the program exit with codes similar to diff(1). That is, it exits with 1 if there were differences and 0 means no differences.
> ```
> `--exit-code` is not implied. From the docs:
> ```
> --exit-code
> Make the program exit with codes similar to diff(1). That is, it exits with 1 if there were differences and 0 means no differences.
> ```

>From https://git-scm.com/docs/git-diff#Documentation/git-diff.txt-emgitdiffemltoptionsgt--no-index--ltpathgtltpathgt:
```
git diff [<options>] --no-index [--] <path> <path>
This form is to compare the given two paths on the filesystem. You can omit the --no-index option when running the command in a working tree controlled by Git and at least one of the paths points outside the working tree, or when running the command outside a working tree controlled by Git. This form implies --exit-code.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129311