ataylorme / eslint-annotate-action

A GitHub action that takes ESLint results from a JSON file and adds them as annotated pull request comments
MIT License
93 stars 33 forks source link

Feature Request: Multiple Reports #21

Closed RBrNx closed 1 year ago

RBrNx commented 3 years ago

Thanks for creating this action! I am currently using it in a monorepo where I have 2 different projects, each of which have their own Github Workflows for linting when a PR is created. Here's an example of one of the workflow files, the other is almost identical except from the paths/working-directory.

name: Lint Mobile Client on PR
on:
  pull_request:
    branches:
      - develop
      - staging
      - production
    paths:
      - 'mobile-client/**'
defaults:
  run:
    working-directory: ./mobile-client
jobs:
  lint:
    name: Lint Mobile Client
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - name: Install modules
        run: yarn
      - name: Save Code Linting Report JSON
        run: yarn lint:report
        continue-on-error: true
      - name: Annotate Code Linting Results
        uses: ataylorme/eslint-annotate-action@1.2.0
        with:
          repo-token: ${{ secrets.GITHUB_TOKEN }}
          report-json: 'eslint_report.json'
          only-pr-files: false

If I create a PR that has file changes to both projects, currently both workflows are triggered as expected and produce the ESLint results as a JSON file. Unfortunately it seems that one set of annotations overwrites the other set, so I only see half of the ESLint issues I'd expect to see. (Notice the missing no-unused-vars error from mobile-client/src/App.js)

image

I'm not sure if that's a limitation of Github Actions/Checks, or if it's something that could be supported by this action?


I think for now I can get around it by having a single workflow lint both projects at the same time to avoid the annotations being overwritten.

As a sidenote I also seem to be getting the following warning, even though the only-pr-files input is documented in the README?

image

Cretezy commented 3 years ago

Ditto, this is a must!

Currently, it only uses the last report. Ideally, it should use all reports

oxc commented 2 years ago

I believe this might be related to #35

ataylorme commented 2 years ago

Hi all, I am making updates in a v2 refactor #44.

As part of this I added a new optional input check-name. The thought is that by differentiating the status check name for each invocation of the Action they won't overwrite each other.

Can you test ataylorme/eslint-annotate-action@v2? You can see the updated README at https://github.com/ataylorme/eslint-annotate-action/tree/v2

ataylorme commented 2 years ago

You can also try combining reports like this comment in #38

jason-yolabs commented 2 years ago

@ataylorme in our case, we're dealing with reports generated across multiple Workflows (not within the same Workflow). In any case, would love to kick the tires on v2 when it's ready since check-name sounds exactly what I'm in need of 😃

ataylorme commented 2 years ago

@jason-yolabs v2 is ready to go now (it has been merged). You can update the version in your workflow files to v2 to use it