-
Notifications
You must be signed in to change notification settings - Fork 934
Update build optimizations #6433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Claude finished @david-livefront's task in 2m 2s —— View job Code Review SummaryStatus: Approved OverviewThis PR consolidates duplicate test task configuration from 9 individual module Changes Reviewed
Key Details
Verification
No issues identified. 🤖 Generated with Claude Code |
| tasks.withType<Test>().configureEach { | ||
| useJUnitPlatform() | ||
| @Suppress("MagicNumber") | ||
| forkEvery = 500 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have moved the test configs to this singular spot for convenience but there are also some small changes too.
- I have increased the
forkEverycount to avoid spinning up so many JVM instances - I have completely removed the
maxHeapSizeproperty to allow the jvm args from thegradle.propertiesto handle it (-Xmx4g).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! @LRNcardozoWDF this may help the extra resource consumption you were noticing.
gradle.properties
Outdated
| org.gradle.caching=true | ||
| org.gradle.configuration-cache=true | ||
| org.gradle.configuration-cache.parallel=true | ||
| org.gradle.configuration-cache.problems=warn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required for some warnings coming from the configuration on release builds.
Thoughts on if we should just remove the configuration cache completely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the configuration cache completely will probably increase build times, so I'm hesitant to go that route. What are the warnings you are seeing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can see the issues here: https://github.com/bitwarden/android/actions/runs/21452527392/job/61788569410
|
Great job! No new security vulnerabilities introduced in this pull request |
9a26530 to
279ba56
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6433 +/- ##
=======================================
Coverage 85.77% 85.77%
=======================================
Files 775 775
Lines 56203 56203
Branches 8123 8123
=======================================
Hits 48207 48207
Misses 5172 5172
Partials 2824 2824 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|

🎟️ Tracking
N/A
📔 Objective
A minor update to simplify parts of the build scripts and improve performance in builds.
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes