EPMatt / reviewdog-action-tsc

Run tsc with reviewdog :dog:
MIT License
19 stars 6 forks source link

`tsc_flags` option wrong default value #39

Closed EPMatt closed 2 years ago

EPMatt commented 2 years ago

I think I encountered similar problems with @MoMoWongHK here a few days ago, but wasn't able to share it until now. When I looked at the GitHub Actions output, it was like this, instead of real TypeScript errors. image I struggled to find what could be the reason, and then found that at the line below, the tsc_flags are defaulted to '.' which would mean that tsc . is executed instead of tsc when tsc_flags is not set. https://github.com/EPMatt/reviewdog-action-tsc/blob/fcd00d5f64b057f2a119879a45b3edcb5c8a83fa/script.sh#L30

I was able to solve that problem by explicitly setting tsc_flags to ' ' (it should have a whitespace or it will be ignored), however the results were not same as in the README. image

Originally posted by @jihoon416 in https://github.com/EPMatt/reviewdog-action-tsc/issues/38#issuecomment-1029646656

EPMatt commented 2 years ago

Hi @jihoon416,

I struggled to find what could be the reason, and then found that at the line below, the tsc_flags are defaulted to '.' which would mean that tsc . is executed instead of tsc when tsc_flags is not set.

Thanks for spotting the bug! That's definitely a typo since the default value at line 30 should be set to '', as stated in the action docs. Would you mind submitting a PR for this? :)

I was able to solve that problem by explicitly setting tsc_flags to ' ' (it should have a whitespace or it will be ignored), however the results were not same as in the README.

This might be an issue with the action config. Could you please share the YAML for the GitHub Actions workflow you're trying to setup?

Thanks!

jihoon416 commented 2 years ago

Hello @EPMatt Thanks for creating a separete issue. Yep I'll get on to submitting a PR!

Here's my full YAML file.

name: Checks

on: pull_request

jobs:
  ESLint:
    runs-on: ubuntu-latest
    steps:
      - name: 🏗 Setup repo
        uses: actions/checkout@v2

      - name: 🏗 Setup Node
        uses: actions/setup-node@v2
        with:
          node-version: 16.x
          cache: yarn

      - name: 📦 Install dependencies
        run: yarn

      - name: 🐶 Activate Review Dog
        uses: reviewdog/action-eslint@v1
        with:
          fail_on_error: true

  Typescript:
    runs-on: ubuntu-latest
    steps:
      - name: 🏗 Setup repo
        uses: actions/checkout@v2

      - name: 🏗 Setup Node
        uses: actions/setup-node@v2
        with:
          node-version: 16.x
          cache: yarn

      - name: 📦 Install dependencies
        run: yarn

      - name: 🐶 Activate Review Dog
        uses: EPMatt/reviewdog-action-tsc@v1
        with:
          tsc_flags: ' '
          fail_on_error: true

It performs two jobs, one for ESLint and one for tsc. ESLint is working fine, but for tsc the result was as below. tsc action Screenshot

EPMatt commented 2 years ago

Hi @jihoon416,

I've just published a release which includes your changes. Thanks again! 🚀 Your YAML config looks good to me. Please be aware that by default reviewdog only reports findings in the added/modified lines in the commits which the check is applied to.

If you want to alter such behavior, you should tweak the action's filter_mode parameter. You can read more about such option in the reviewdog documentation.

Thank you! :)

jihoon416 commented 2 years ago

Thanks for the detailed explanation! Maybe that was why, I'll try again when I have time.

jihoon416 commented 2 years ago

Hi @EPMatt I just tried changing the filter_mode parameter, and it worked! Thanks a lot. I should've looked into the reviewdog documentation more 😅 I've decided to use nofilter, because changing things in one file usually affect the tsc result of other files as well.

Would it be possible to add something similar to the following to the FAQ? I think people new to reviewdog like me might also be confused at first.

### Why can't I see the results?
Try looking into the `filter_mode` options explained [here](https://github.com/reviewdog/reviewdog#filter-mode). Typescript errors will sometimes appear in lines or files that weren't modified, which don't get filtered with the default `added` option.
EPMatt commented 2 years ago

Hi @jihoon416,

thank you for the reply. I'm glad that your issue is solved. :)

Would it be possible to add something similar to the following to the FAQ?

That's a good idea! If you agree with this, I'd just tweak the FAQ as follows:

...in lines or files that weren't modified by the commit the workflow run is associated with, which instead get filtered with the default added option.

Would you mind submitting a PR for this too? :)

Thank you for your suggestion!

jihoon416 commented 2 years ago

Hello @EPMatt

I made PR #46 with your suggestion applied as well. Thanks a lot for taking interest and making me able to participate! 😃

EPMatt commented 2 years ago

Hi @jihoon416,

I'm happy to help you participating to open source, and glad that you chose my repo for your first contributions! 🚀 Thank you so much for your time. :)

github-actions[bot] commented 2 years ago

Hi there,

🔒 This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new ticket for related bugs.

Thanks!