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: Allow annotation on push #18

Closed elstgav closed 1 year ago

elstgav commented 3 years ago

Thanks for creating this! The action is working great for my team, except we’d like to annotate on push instead of pull_request, and disable the report output.

We often work in separate branches and open a pull request later in development—that means relying on push for early test/lint feedback, before we get to opening a PR. However, once we open a PR, inline annotation is super helpful 👍🏻.

As it stands, that means enabling both push and pull_request, which means the linter runs twice for every commit once we have a PR open 👎🏻 .

We also use rubocop-annotate-action which allows us to do what I’m talking about—we have it set to run on push only, and it will annotate PRs when they’re opened. I’m hoping we can do the same with eslint-annotate-action!

ataylorme commented 3 years ago

The action is working great for my team, except we’d like to annotate on push instead of pull_request, and disable the report output.

Hi @elstgav I am not clear on the use case. This Action was specifically made to do the annotation portion only -- you must still run ESLint.

I would suggest you run ESLint on its own for push events, giving you feedback on if linting passes, and another workflow on the pull_request event that runs ESLint with the JSON report and annotations.

You can do this by creating two workflow files, one for push and one for pull_request, or use the conditional syntax to only run this action when the event is pull_request.

That might look something like this:

name: Example NodeJS Workflow

on: [pull_request, push]

jobs:
  node_test:
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v2
      - name: Node.JS 14
        uses: actions/setup-node@v2
        with:
          node-version: 14
      - name: Cache node modules
        uses: actions/cache@v1
        with:
          path: ~/.npm
          key: ${{ runner.OS }}-build-${{ hashFiles('**/package-lock.json') }}
          restore-keys: |
            ${{ runner.OS }}-build-${{ env.cache-name }}-
            ${{ runner.OS }}-build-
            ${{ runner.OS }}-
      - name: Install Node Dependencies
        run: npm install
        env:
          CI: TRUE
      - name: Test Code Linting
        run: npm run lint
      - name: Save Code Linting Report JSON
        # Only run lint reports on pull requests
        if: github.event_name == 'pull_request'
        # npm script for ESLint
        # eslint --output-file eslint_report.json --format json src
        # See https://eslint.org/docs/user-guide/command-line-interface#options
        run: npm run lint:report
        # Continue to the next step even if this fails
        continue-on-error: true
      - name: Annotate Code Linting Results
        # Only run lint annotations on pull requests
        if: github.event_name == 'pull_request'
        uses: ataylorme/eslint-annotate-action@1.1.3
        with:
          repo-token: "${{ secrets.GITHUB_TOKEN }}"
          report-json: "eslint_report.json"
      - name: Upload ESLint report
        # Only upload ESLint reports for pull requests
        if: github.event_name == 'pull_request'
        uses: actions/upload-artifact@v2
        with:
          name: eslint_report.json
          path: eslint_report.json
elstgav commented 3 years ago

@ataylorme yeah, I think we’re talking about something different. I wasn’t talking about how eslint is run, simply the fact that eslint-annotate-action will only add annotations for pull_request events.

Like I mentioned, rubocop-annotate-action works much the same way, (run the rubocop linter on your own, and then it reports on the output), except it will do inline annotations on either push or pull_request events.

For some projects, a team may only want linting running on push requests, and eslint-annotate-action only lets them upload reports—I don’t see why it can’t do inline annotation as well, and make that user-configurable.

ataylorme commented 2 years ago

@elstgav I see your use case now but unfortunately I don't have the time to add that feature. If you would like to contribute to add it please do so on the v2 branch.