[rewrite] Add GradleBestPractices
#19885
Conversation
Pankraz76
force-pushed
the
rewrite-fix-gradle
branch
from
e9e50d6 to
4d3b181
Compare
|
❌ Gradle check result for 4d3b181: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Pankraz76
force-pushed
the
rewrite-fix-gradle
branch
2 times, most recently
from
575268d to
6e63234
Compare
| distribution: temurin | ||
| cache: gradle | ||
| - name: Run Gradle (precommit) | ||
| - name: Run Gradle (pre-commit) |
There was a problem hiding this comment.
could kind of suggest this, also to be aligned with new check.
Pankraz76
force-pushed
the
rewrite-fix-gradle
branch
from
6e63234 to
cda8eff
Compare
|
|
||
| public void testMaxProcesses() throws IOException, InterruptedException { | ||
| String limits = executeCommand("sudo su -c 'cat /proc/" + opensearchPid + "/limits'", "Failed to read process limits"); | ||
| assertTrue("Max processes limit should be 4096 or unlimited", |
There was a problem hiding this comment.
also spotless is not spotless, like anything else made by error-prone human being. mea culpa.
| - name: Run Gradle (sanity-check) | ||
| continue-on-error: ${{ matrix.experimental }} | ||
| shell: bash | ||
| run: ./gradlew rewriteDryRun -Dorg.gradle.jvmargs=-Xmx8G |
There was a problem hiding this comment.
assuming we can reuse cached compile phase somehow.
is this actual true? kindly request @timtebeek 🍀
There was a problem hiding this comment.
Don't know; haven't tried. For large projects like this one I'd advise against running against every pull request, and instead run periodically against pre-built serialized LSTs through Moderne, as it can take a long time to run here.
There was a problem hiding this comment.
Thanks for the update. So, this means we have to rely on an externally hosted Moderne instance, right?
Yes, building on every commit doesn’t seem ideal. On the other hand, it would also be unpleasant to fix unrelated issues, and these kinds of diffs can be quite large since they don’t run on every increment. Of course, it’s very resource-intensive, but the tradeoff favors maintaining a clean state over minimizing CI costs.
The good news is that it’s running quite fast now — around 15 minutes. Six months ago, it took about 45 minutes, so there’s already been significant improvement. The sanity check also runs much faster than the current pre-commit goal, wondering what in detail it does given the long execution time.
Currently, this is handled similarly in Checkstyle and Spotless — we run and check on every increment, assuming we’re on the happy path with minimal or no changes. If that’s the case, it actually supports your approach of running these checks only occasionally rather than continuously.
However, assuming we need to relying on external resources, I’d suggest taking the strict approach for now — checking on every increment — leaving any pipeline optimization or downsizing for the future.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #19885 +/- ##
============================================
- Coverage 73.21% 73.16% -0.05%
+ Complexity 71254 71182 -72
============================================
Files 5766 5766
Lines 325470 325470
Branches 47084 47084
============================================
- Hits 238296 238142 -154
- Misses 68043 68148 +105
- Partials 19131 19180 +49 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Vincent Potucek <vpotucek@me.com>
Pankraz76
force-pushed
the
rewrite-fix-gradle
branch
from
cda8eff to
85af591
Compare
|
This might be an easy starter. Rewrite has evolved, passing much quicker. kindly request your feedback, thanks. |
2 participants
Description
[Describe what this change achieves]
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.
follow up from:
26m 56s vs 17m 23s