action-rubocop icon indicating copy to clipboard operation
action-rubocop copied to clipboard

add only_changed input to run rubocop only against changed files

Open toy opened this issue 9 months ago • 5 comments

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 avatar May 02 '24 18:05 toy

@javierjulio May I ping you for input?

toy avatar May 23 '24 16:05 toy

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

haya14busa avatar Jun 25 '24 13:06 haya14busa

@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

toy avatar Jun 25 '24 14:06 toy

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 avatar Jun 26 '24 10:06 haya14busa

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.

haya14busa avatar Jun 26 '24 10:06 haya14busa

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

v-kumar avatar Jul 12 '24 13:07 v-kumar

@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 avatar Jul 12 '24 13:07 toy

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

toy avatar Jul 12 '24 20:07 toy

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!

review-dog avatar Jul 14 '24 12:07 review-dog

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

github-actions[bot] avatar Jul 14 '24 12:07 github-actions[bot]