Support ignore deletions with "ignore_line_deletions" param #71
Conversation
johnlk
changed the title
|
Could we have two settings:
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 |
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
force-pushed
the
john/allow-ignoring-deletion
branch
from
ee92b38
to
e7ddbea
Compare
Test files_to_ignore code path Tweak the variable type Correct bad logic in the file ignore block Debug More debugging More debugging Fingers crossed Another try Last try Lets see if this one does it Testing again More tests More testing Last test Wait, I might be onto something Put some changes back Fixes Whoops Walk back yml test
johnlk
force-pushed
the
john/allow-ignoring-deletion
branch
from
e7ddbea
to
c6dc97a
Compare
johnlk
changed the title
|
@OnkarRuikar to prevent this PR from getting any larger, I'm going to implement the This current change has test updates, refactoring, and a new feature, so I want to stop here for now. |
| if [[ "$pull_request_number" != "null" ]]; then | ||
| echo "$pull_request_number" | ||
| else | ||
| echo "Not a pull request event" |
There was a problem hiding this comment.
this code block would've saved me so much time while I was testing this. I was testing on push events and was getting very obscure errors. 🙃
johnlk
changed the title
src/github.sh
Outdated
| _jq() { | ||
| echo ${file} | base64 -d | jq -r ${1} | ||
| } |
There was a problem hiding this comment.
Why not reusing jq::base64 instead?
There was a problem hiding this comment.
Oh shoot, didn't realize this helper was already available. Will use!
Co-authored-by: Javier Ferrer González <[email protected]>
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_deletionswhich defaults tofalse.By request I may add another variable
ignore_file_deletionswhich 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
changesvariable in the PR files loop intoadditionsanddeletions. This makes the code more consistent between the if and else blocks in thecalculate_total_modificationsmethod.How to test
I ensured that all four possible code paths were tested.
files_to_ignore(set or not) xignore_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