errata-ai / vale-action

:octocat: The official GitHub Action for Vale -- install, manage, and run Vale with ease.
MIT License
202 stars 51 forks source link

Refine how `onlyAnnotateModifiedLines` should work #32

Closed jdkato closed 2 years ago

jdkato commented 3 years ago

It's not just the last commit; it's every commit individually.

It seems like you'd like it to be every commit collectively. In other words, every commit would also include all annotations from all previous commits.

This is possible, but less straightforward since the it only applies to PRs (whereas the the current functionality can apply to pushes too) and the content of a line can change between commits.

For now, I think just using files: __onlyModified is closer to what you want. But we can discuss this more, although I think I'll move it to a new issue.

Originally posted by @jdkato in https://github.com/errata-ai/vale-action/issues/27#issuecomment-772625189

jouni commented 3 years ago

It's not just the last commit; it's every commit individually.

I think that could be fine, if the PR check would fail if one of those commits has an error, not just if the last commit has an error. It would be slightly more work to browse through commits to find the cause, but I think that would be acceptable.

every commit would also include all annotations from all previous commits.

This would be ideal for PRs.

tjperry07 commented 2 years ago

I did some long term testing and found that you can't use onlyAnnotateModifiedLines: true along with files. It took me a bit to figure it out.

If both files and onlyAnnotateModifiedLines are present, it doesn't lint anything and returns passing for the checks. If I remove onlyAnnotateModifiedLines, then it of course checks everything in the PR which is not what I expected.

I expected it to only check the files in the folder specified and only check lines that were annotated. I have multiple markdown folders but only need one folder checked.

Does not work

      - name: Vale
        uses: errata-ai/vale-action@v1.4.2
        with:
          files: src/docs/
          onlyAnnotateModifiedLines: true
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
CollierCZ commented 2 years ago

When I tested it (in this PR), it seemed to ignore the files directive completely, but still fail on errors in modified lines.

jdkato commented 2 years ago

Thanks for the reports.

I'm a little behind with maintaining this repo, but I plan on getting a new release out in early 2022 that should, at the least, allow files to work with onlyAnnotateModifiedLines.

jdkato commented 2 years ago

You can now control this with the filter_mode option when using the @reviewdog version:

name: reviewdog
on: [pull_request]

jobs:
  vale:
    name: runner / vale
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - uses: errata-ai/vale-action@reviewdog
        with:
          filter_mode: added
        env:
          GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}