Add Danger check to discourage large PRs

[tonidero]

Why

Large PRs are slow and error-prone to review, and problems are more likely to slip through. This adds a Danger check that nudges us toward smaller, more focused PRs.

What

Fails the Danger check when a PR changes more than 300 lines of production Kotlin/Java code (insertions + deletions). Test sources and generated/build output are excluded so only "real" code counts.

When a large PR is genuinely necessary, add the skip-pr-lines-changed-check label to bypass the failure.

---

Note

Low Risk
CI-only Danger policy in Dangerfile; no runtime or app code changes, with an explicit bypass label for exceptions.

Overview
Adds a Danger CI gate that fails pull requests when combined insertions and deletions in production .kt / .java files exceed 300 lines.

Churn is summed only for non-test, non-generated paths (excludes src/test, androidTest, testFixtures, build, generated, and *Test(s).kt/java). PRs that must stay large can bypass with the skip-pr-lines-changed-check label, which turns the failure into an informational message.

^{Reviewed by Cursor Bugbot for commit 95c2c7f. Bugbot is set up for automated code reviews on this repo. Configure here.}

[tonidero]

Note how in the end, decided to exclude tests... I do think tests are very important, but I'm afraid that if we include tests in the count, we might write less tests...

[rickvdl]

I see /purchases/src/testDefaults/.. (same for CEC) wouldn't match the directory check, but it would match the files as long as they end with Test(s).kt/.java so I think we're good. Helper files etc would't be caught, but probably not the biggest deal.

[tonidero]

Good points! Those folders are the minority, since normally most code is part of the common source set. But it's true it's something we will need to keep an eye out in the future if we add more flavors.

[tonidero]

Not including gradle files or anything like that... Lmk if you have any thoughts on this!

[tonidero]

Set the limit to 300 right now, which, without including tests, I think it's already pretty big... Lmk if we should aim for lower amounts.

[rickvdl]

Just checked a few PRs for reference, I think it's a reasonable start and let's tweak if needed :)

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 17551f2. Configure here.}

[cursor]

Deleted production files omitted

Medium Severity

The production line budget only iterates git.modified_files and git.added_files, so wholly removed production .kt/.java files never enter prod_code_files. Their deletions are omitted from total_changed, so large delete-only or delete-heavy refactors can stay under the 300-line cap despite matching the stated insertions-plus-deletions goal.

 

^{Reviewed by Cursor Bugbot for commit 17551f2. Configure here.}

[tonidero]

Hmm this might be ok? I guess it does depend on the file in the end and how much logic it has... But I think adding this might discourage refactors...

[rickvdl]

That's fine I'd say, deletions are a lot easier to review as well.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.28%. Comparing base (dd728df) to head (95c2c7f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3543   +/-   ##
=======================================
  Coverage   80.28%   80.28%           
=======================================
  Files         377      377           
  Lines       15386    15386           
  Branches     2134     2134           
=======================================
  Hits        12353    12353           
  Misses       2174     2174           
  Partials      859      859           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rickvdl]

Great to have this 💪

[rickvdl]

Just checked a few PRs for reference, I think it's a reasonable start and let's tweak if needed :)

[rickvdl]

That's fine I'd say, deletions are a lot easier to review as well.

[rickvdl]

I see /purchases/src/testDefaults/.. (same for CEC) wouldn't match the directory check, but it would match the files as long as they end with Test(s).kt/.java so I think we're good. Helper files etc would't be caught, but probably not the biggest deal.

[ajpallares]

Thanks for adding this! Once included in all SDK repos, we should add the new skip-pr-lines-changed-check label to our internal SDK GitHub labels notion page, for reference

[AlvaroBrey]

Nice! Single question here: have you considered adding this to the shared Dangerfile instead?

Especially since we already have check_pr_size_increase (bytes) there

[tonidero]

I thought about it... My main reason to keep it split in each repo is that there are different files/folders we need to analyze on each SDK. Additionally, I think we might want to have a somewhat smaller diff max on some hybrids since most logic lives in the natives, ideally not such big PRs are needed in the hybrids. Of course, we can revisit this later if needed :)