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

feat(action-options): add tool_name option for reviewdog reporter #30

Merged
merged 1 commit into from
May 12, 2020

Conversation

mihailpozarski
Copy link
Contributor

Hey reviewdog team! thanks for this awesome action!

I implemented it on a private project to take care of the javascript guidelines and I notice a use case that it's not supported at the time, as it is a little wired for small projects, but not that uncommon. The project is a rails 4.2 app with Angular and its transitioning from ES5 on the assets pipeline to ES6 with webpacker. As a result there are 2 javascripts directories, app/assets/javascript for old files or non-migrated files and app/javascriptfor the ES6-webpacker part of the project. For this reason, i had to had two steps on my reviewdog check for eslint:

name: reviewdog
on: [pull_request]
jobs:
  eslint:
    name: eslint(ES5)
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v1
      - name: eslint(ES5)
        uses: reviewdog/action-eslint@v1
        with:
          github_token: ${{ secrets.github_token }}
          reporter: github-pr-check
          workdir: 'app/assets/javascripts'
      - name: eslint(ES6)
        uses: review/action-eslint@master
        with:
          github_token: ${{ secrets.github_token }}
          reporter: github-pr-check
          workdir: 'app/javascript'

the problem is that as the action uses "eslint" as de name of the reporter for both checks, the first report is ignored and only the second one is used.

I managed to solve this by adding the tool_name option when the action executes reviewdog (I stole the idea from your reviewdog/action-rubocop repo)

Hope this PR is well recieved and become useful for someone else!

@tadjohnston
Copy link

I had a similar issue for action-stylelint and did the same here reviewdog/action-stylelint#14. Maybe we can either rename this one or that one to keep it consistent for the naming convention?

@mihailpozarski
Copy link
Contributor Author

@tadjohnston I agree, I think we could use tool_name for reducing the PRs needed as reviewdog/action-golangci-lint, reviewdog/action-brakeman, reviewdog/action-reek, reviewdog/action-rubocop already use it

Copy link
Member

@haya14busa haya14busa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!!

@haya14busa haya14busa merged commit 9df36a0 into reviewdog:master May 12, 2020
@github-actions
Copy link
Contributor

🚀 [bumpr] Bumped! New version:v1.7.0 Changes:v1.6.0...v1.7.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants