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

intuitive action exit code #7

Closed
2bndy5 opened this issue Aug 18, 2021 · 1 comment · Fixed by #10
Closed

intuitive action exit code #7

2bndy5 opened this issue Aug 18, 2021 · 1 comment · Fixed by #10
Assignees
Labels
enhancement New feature or request

Comments

@2bndy5
Copy link
Collaborator

2bndy5 commented Aug 18, 2021

As it is currently, the action current exits with a successful code (0) even when the run_check.sh finds problems given by clang-tidy or clang-format.

Solution

toward the end of the script, add

EXIT_CODE=0
if [ "$PAYLOAD_FORMAT" != "" && "$PAYLOAD_TIDY" != "" ]
then
  EXIT_CODE=1
fi
exit($EXIT_CODE)

Counter-argument

If the intention is to allow PRs to merge errors (ignore the warning from clang-*), then the user's workflow could handle the exit code instead of hard-coding it in the run_checks.sh. This option would require a output from this action:

jobs:
  test:
    runs-on: ubuntu-latest
    steps:
      - uses: shenxianpeng/cpp-linter-action@master
        id: linter
        with:
          style: file
          extensions: 'cpp'
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

      # If there are any comments, fail the check
      - if: steps.linter.outputs.checks-failed > 0
        run: exit 1

This would simply require a change to the above bash script suggestion

EXIT_CODE=0
if [ "$PAYLOAD_FORMAT" != "" && "$PAYLOAD_TIDY" != "" ]
then
  EXIT_CODE=1
fi
# exit($EXIT_CODE) # let action exit successfully

# set the action's `checks-failed` output
echo "::set-output name=checks-failed::$(echo $EXIT_CODE)"
@2bndy5 2bndy5 changed the title allow action to fail if some files were not formatted properly intuitive action exit code Aug 18, 2021
@shenxianpeng
Copy link
Collaborator

Cool 👍 your idea makes this linter action be more useful in real projects.

@shenxianpeng shenxianpeng added the enhancement New feature or request label Aug 19, 2021
@2bndy5 2bndy5 self-assigned this Aug 19, 2021
2bndy5 added a commit to 2bndy5/cpp-linter-action that referenced this issue Aug 19, 2021
@2bndy5 2bndy5 mentioned this issue Aug 19, 2021
shenxianpeng added a commit that referenced this issue Aug 20, 2021
* solution for #7
* parameterize EXIT_CODE
* fix bash function calls; switch to the new docker image
* fix bash array length syntax
* oops need that apt update
* ok now change the test code a bit
* check output is working
* fix bad yml syntax
* is it being set at all?
* use a string instead of an integer
* fix bash logic syntax
* single square brackets are the old way to do logic
* review changes
* add output var info to the readme
* Update demo.cpp
* Revert action to upstream master

Co-authored-by: shenxianpeng <xianpeng.shen@gmail.com>
2bndy5 added a commit to 2bndy5/cpp-linter-action that referenced this issue Aug 20, 2021
shenxianpeng pushed a commit that referenced this issue Aug 22, 2021
* Output exit code (#10)
* solution for #7
* parameterize EXIT_CODE
* fix bash function calls; switch to the new docker image
* fix bash array length syntax
* oops need that apt update
* ok now change the test code a bit
* check output is working
* fix bad yml syntax
* is it being set at all?
* use a string instead of an integer
* fix bash logic syntax
* single square brackets are the old way to do logic
* review changes
* add output var info to the readme
* Update demo.cpp
* Revert action to upstream master
* fix dockerfile from image not exist
* solution for #7
* parameterize EXIT_CODE
* fix bash function calls; switch to new docker image
* check output is working
* show me some env vars
* trigger on push events
* switch action to push-capable branch
* echo the JSON that lives on the runner
* use cat instead of echo
* remove backticks
* use alpine while debugging git events
* Revert "use alpine while debugging git events"
This reverts commit fbbb1b5a6f8a80f8d4245ebce07d7048ff74fedd.
* test push compatibility
* provide version input
* docker uses tags
* oops update run_checks.sh
* fix jq args on push
* test push src change
* test src change w/ GH checkout
* fix filename output for push events (w/ checkouts)
* test new comment body
* gotta break that bad habit
* check against src change
* use sed directly instead of sed then cat
* tidy docs lied
* don't know why sed isn't working
* try multiple ARG options
* try to replace the docker file
* maybe without dyn var
* bad domain?
* nope needs a docker:// domain
* reverting changes to docker file FROM
* fix clang-format output to file
* test on src changes
* list clang-fmt warnings per file
* add demo.h and adjust comment body
* make test action use default extensions
* don't checkout repo on test action
* stay in working dir. DL into a hidden file
* organize output better
* adjust output again
* fix output; switch back to checkout repo
* enforce CPP syntax in the filename (no checkout)
* sed taking too much out of the path in clang-tidy out
* that's better (checkout is faster)
* segregate script into functions
* oops uncomment from debugging locally
* show me a long diff
* show me a split diff
* clean-up (ready for PR)
* update readme
* remove the version from user inputs
* only apply clang-tidy checks if it was specified
* switch test action back to upstream
* add version for user input
shenxianpeng added a commit that referenced this issue Aug 22, 2021
* #8 update readme to suggest open push event by default

* Push capable (#14)

* Output exit code (#10)
* solution for #7
* parameterize EXIT_CODE
* fix bash function calls; switch to the new docker image
* fix bash array length syntax
* oops need that apt update
* ok now change the test code a bit
* check output is working
* fix bad yml syntax
* is it being set at all?
* use a string instead of an integer
* fix bash logic syntax
* single square brackets are the old way to do logic
* review changes
* add output var info to the readme
* Update demo.cpp
* Revert action to upstream master
* fix dockerfile from image not exist
* solution for #7
* parameterize EXIT_CODE
* fix bash function calls; switch to new docker image
* check output is working
* show me some env vars
* trigger on push events
* switch action to push-capable branch
* echo the JSON that lives on the runner
* use cat instead of echo
* remove backticks
* use alpine while debugging git events
* Revert "use alpine while debugging git events"
This reverts commit fbbb1b5a6f8a80f8d4245ebce07d7048ff74fedd.
* test push compatibility
* provide version input
* docker uses tags
* oops update run_checks.sh
* fix jq args on push
* test push src change
* test src change w/ GH checkout
* fix filename output for push events (w/ checkouts)
* test new comment body
* gotta break that bad habit
* check against src change
* use sed directly instead of sed then cat
* tidy docs lied
* don't know why sed isn't working
* try multiple ARG options
* try to replace the docker file
* maybe without dyn var
* bad domain?
* nope needs a docker:// domain
* reverting changes to docker file FROM
* fix clang-format output to file
* test on src changes
* list clang-fmt warnings per file
* add demo.h and adjust comment body
* make test action use default extensions
* don't checkout repo on test action
* stay in working dir. DL into a hidden file
* organize output better
* adjust output again
* fix output; switch back to checkout repo
* enforce CPP syntax in the filename (no checkout)
* sed taking too much out of the path in clang-tidy out
* that's better (checkout is faster)
* segregate script into functions
* oops uncomment from debugging locally
* show me a long diff
* show me a split diff
* clean-up (ready for PR)
* update readme
* remove the version from user inputs
* only apply clang-tidy checks if it was specified
* switch test action back to upstream
* add version for user input

Co-authored-by: Brendan <2bndy5@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants