search

CodelyTV / pr-size-labeler

🏷 Visualize and optionally limit the size of your Pull Requests
https://github.com/marketplace/actions/pull-request-size-labeler
MIT License
336 stars 58 forks source link

Support ignore deletions with "ignore_line_deletions" param #71

Closed johnlk closed 5 months ago

johnlk commented 5 months ago

What type of PR is this? (check all applicable)

Description

Some users prefer to not include file line deletions in the total line change count. I'm allowing this by way of a new config variable ignore_line_deletions which defaults to false.

By request I may add another variable ignore_file_deletions which would not count files which are completely deleted. This will most likely come as a followup change.

Notes to Reviewer

This required I break up the changes variable in the PR files loop into additions and deletions. This makes the code more consistent between the if and else blocks in the calculate_total_modifications method.

How to test

I ensured that all four possible code paths were tested. files_to_ignore (set or not) x ignore_line_deletions (true/false).

In the process of adding these tests, I added an example response for the pull request API. This was missing.

Link to issues addressed

OnkarRuikar commented 5 months ago

Could we have two settings:

  1. ignore_file_deletions: Ignore lines from only deleted files but count deleted lines from modified files.
  2. ignore_line_deletions: ignore all the deleted lines. This would be super set of ignore_file_deletions which would ignore deleted files from all kinds of modifications.

Depending on the project, line deletions from modified files does require review efforts, so we do need to count lines from modified files but not from deleted files. In such case ignore_line_deletions won't cut it and ignore_file_deletions would be more precise.

johnlk commented 5 months ago

Could we have two settings:

  1. ignore_file_deletions: Ignore lines from only deleted files but count deleted lines from modified files.
  2. ignore_line_deletions: ignore all the deleted lines. This would be super set of ignore_file_deletions which would ignore deleted files from all kinds of modifications.

Depending on the project, line deletions from modified files does require review efforts, so we do need to count lines from modified files but not from deleted files. In such case ignore_line_deletions won't cut it and ignore_file_deletions would be more precise.

Thanks for the feedback, I think this is a great distinction and something I can incorporate. I could see why ignoring file deletions alone could prove valuable.

johnlk commented 5 months ago

@OnkarRuikar to prevent this PR from getting any larger, I'm going to implement the ignore_file_deletions option in a followup PR.

This current change has test updates, refactoring, and a new feature, so I want to stop here for now.