search

reviewdog / action-rubocop

Run rubocop with reviewdog 🐶
MIT License
114 stars 75 forks source link

add only_changed input to run rubocop only against changed files #103

Closed toy closed 2 months ago

toy commented 4 months ago

Add an input to limit running rubocop only against files changed in the PR.

Had to switch to bash for process substitution and working with arrays.

Ignore if there are more than 100 changed files. Main reason is to protect from possibly hitting the command line length limit. In reality the command line length limit most probably (depends on OS) allows much higher number of files and if needed can be calculated or provided as one more input.

Fix ci workflow to fetch all commits for pr branch plus head commit of base branch to be able to find changed files.

~Will conflict with #102, I will update whichever is not merged first~

toy commented 4 months ago

@javierjulio May I ping you for input?

haya14busa commented 2 months ago

Can you add a test? https://github.com/reviewdog/action-rubocop/pull/103

toy commented 2 months ago

@haya14busa Can you suggest how/what to test? I can think of either adding test branches and test workflows to check complete thing, or just unit testing script.sh similar to how it is done in test/rdjson_formatter/test.sh

haya14busa commented 2 months ago

I think we can add a job (or workflow) with only_changed=true and add new test files under test dir.

We can at least check the behavior with this pr. As you mentioned it, it would be cool if we can prepare a test branch too, but it's optional.

haya14busa commented 2 months ago

Ideally, it would be great if we can use the existing solution like https://github.com/tj-actions/changed-files for getting changed files so that we can reuse the similar solution for other reviewdog actions.

But we use robocop specific command (rubocop --list-target-files ), so it's optional.

v-kumar commented 2 months ago

Any update on this? Would love to see this merged.

toy commented 2 months ago

@v-kumar I did an unsuccessful attempt and afterwards came up with a working way to test this, but didn't yet have time. The idea is to test by running the script while mocking binaries by manipulating PATH

toy commented 2 months ago

@haya14busa I added tests of script.sh without using any frameworks

review-dog commented 2 months ago

Hi, @toy! We merged your PR to reviewdog! 🐶 Thank you for your contribution! ✨

We just invited you to join the @reviewdog organization on GitHub. Accept the invite by visiting https://github.com/orgs/reviewdog/invitation. By joining the team, you'll be a part of reviewdog community and can help the maintenance of reviewdog.

Thanks again!

github-actions[bot] commented 2 months ago

🚀 [bumpr] Bumped! New version:v2.17.0 Changes:v2.16.0...v2.17.0