Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inplace option being always overwritten #22

Merged
merged 1 commit into from
Oct 18, 2020

Conversation

schra
Copy link
Contributor

@schra schra commented Oct 17, 2020

Observation

I observed that the following action always passes - even after I
added bad indentation on purpose.

name: lint
on: push
jobs:
  clang-format:
    runs-on: ubuntu-latest
    steps:
    - name: Checkout
      uses: actions/checkout@v2

    - name: clang format
      uses: DoozyX/clang-format-lint-action@v0.10
      with:
        extensions: 'h,cpp'
        clangFormatVersion: 10

For example, the following call always returns an exit code of 0:

docker run -it --rm --workdir /src -v $(pwd):/src clang-format-lint \
  "--clang-format-executable" "/clang-format/clang-format10" "-r" \
  "--inplace" "false" "--extensions" "h,cpp" "--exclude" "none" "."

The bug

Turns out that --inplace was parsed wrong and as soon as --inplace
is passed (and it is always passed) it essentially actually meant
--inplace True. And when --inplace True is passed, then the exit
code will be 0 and thus the action always is successful.

See
https://stackoverflow.com/questions/15008758/parsing-boolean-values-with-argparse
for more information on parsing booleans.

Running the above Docker command with my fix actually returns an exit
code of 1 and thus the actions also fails as expected.

Fixes #18. Fixes #19

Observation
-----------

I observed that the following action **always** passes - even after I
added bad indentation on purpose.

```
name: lint
on: push
jobs:
  clang-format:
    runs-on: ubuntu-latest
    steps:
    - name: Checkout
      uses: actions/checkout@v2

    - name: clang format
      uses: DoozyX/clang-format-lint-action@v0.10
      with:
        extensions: 'h,cpp'
        clangFormatVersion: 10
```

For example, the following call always returns an exit code of 0:

```
docker run -it --rm --workdir /src -v $(pwd):/src clang-format-lint \
  "--clang-format-executable" "/clang-format/clang-format10" "-r" \
  "--inplace" "false" "--extensions" "h,cpp" "--exclude" "none" "."
```

The bug
-------

Turns out that `--inplace` was parsed wrong and as soon as `--inplace`
is passed (and it is always passed) it essentially actually meant
`--inplace True`. And when `--inplace True` is passed, then the exit
code will be 0 and thus the action always is successful.

See
https://stackoverflow.com/questions/15008758/parsing-boolean-values-with-argparse
for more information on parsing booleans.

Running the above Docker command with my fix actually returns an exit
code of 1 and thus the actions also fails as expected.
@DoozyX
Copy link
Owner

DoozyX commented Oct 18, 2020

Thank you

@DoozyX DoozyX merged commit 70c0f4e into DoozyX:master Oct 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lint errors not recognized on version 0.10 inplace=False option doesn't work
2 participants