Make GradleInspector the new default analyzer for Gradle projects #9070

sschuberth · 2024-09-03T10:20:37Z

Please have a look at the individual commit messages for the details.

codecov · 2024-09-03T10:30:45Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.21%. Comparing base (77590e3) to head (c9db5a7).
Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #9070   +/-   ##
=========================================
  Coverage     67.21%   67.21%           
  Complexity     1188     1188           
=========================================
  Files           239      239           
  Lines          7916     7916           
  Branches        910      910           
=========================================
  Hits           5321     5321           
  Misses         2226     2226           
  Partials        369      369

Flag	Coverage Δ
funTest-docker	`60.36% <ø> (ø)`
funTest-non-docker	`34.59% <ø> (ø)`
test	`36.53% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

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

fviernau · 2024-09-03T10:44:35Z

plugins/package-managers/gradle-inspector/README.md


-Also, the `isModified` check which compares with artifacts of the same name in Maven Central is not implemented yet.
+* The retrieval of the checksum values for remote artifacts is currently done via plain OkHttp calls, which means it will not work out of the box for private repositories.
+  To work around this, credentials need to be configured in `.netrc` additionally to in Gradle.


Should this say "in addition to"?

Furthermore, I'm not sure but it felt to me within "to in" there is something missing.

Maybe: The credentials which are configured in Gradle need to be redundantly configured in '.netrc'.

I wanted to keep changes compared to the existing text to a minimum, but I guess I could do some rewordings along the way...

fviernau · 2024-09-03T10:45:39Z

plugins/package-managers/gradle-inspector/README.md

-Also, the `isModified` check which compares with artifacts of the same name in Maven Central is not implemented yet.
+* The retrieval of the checksum values for remote artifacts is currently done via plain OkHttp calls, which means it will not work out of the box for private repositories.
+  To work around this, credentials need to be configured in `.netrc` additionally to in Gradle.
+  This is similar to how the "legacy" [Gradle] analyzer required to additionally configure credentials in Maven.


Would it make sense to drop line 38 as irrelevant in this context?

fviernau · 2024-09-03T10:46:22Z

plugins/package-managers/gradle-inspector/README.md

+* The retrieval of the checksum values for remote artifacts is currently done via plain OkHttp calls, which means it will not work out of the box for private repositories.
+  To work around this, credentials need to be configured in `.netrc` additionally to in Gradle.
+  This is similar to how the "legacy" [Gradle] analyzer required to additionally configure credentials in Maven.
+* The `isModified` check which compares with artifacts of the same name in Maven Central is not implemented yet.


Is there something missing in "which compares with artifacts" ?

I've clarified it.

fviernau · 2024-09-03T10:52:39Z

plugins/package-managers/gradle-inspector/README.md

-The [GradleInspector] is an alternative analyzer for projects that use the Gradle package manager.
-It is supposed to address [several] [shortcomings] of the "legacy" [Gradle] analyzer, but to not interfere with it, the [GradleInspector] is disabled by default.
+The [GradleInspector] is the new analyzer for projects that use the Gradle package manager.
+It is supposed to address [several] [shortcomings] of the "legacy" [Gradle] analyzer, which is disabled by default now.


Would it make sense to avoid "the new" and "now" as these terms become outdated?
Maybe instead "the default" and "which is disabled by default"?

sschuberth · 2024-09-10T12:25:14Z

@oss-review-toolkit/kotlin-devs what's currently blocking the merge of this is some unexpected diff in the dependency graph that I see when running PubFunTest. It seems that some de-duplication is happening for Android / Gradle dependencies. Back here I assumed this was due to the GradleInspector not using the dependency graph yet, but that's not true anymore, so I'm stumped.

sschuberth · 2024-09-10T15:39:24Z

I'm stumped.

I've found the cause and am preparing a fix.

fviernau

Nice!

This is a fixup for 7373195. Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>

In ff0ca15, `OPTION_GRADLE_VERSION` (which equals "gradleVersion") was errorneously replaced with `gradleFactory.type` (which equals "Gradle") in two places. One of these was fixed in 1ba253a. Fix the other one here. Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>

As of 04b0356, the analyzer refuses to have multiple package managers for the same project type enabled. So make Pub always use that one, if any, Gradle package manager to avoid problems if a user enables `GradleInspector` but Pub tries to use legacy `Gradle`. Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>

Format limitations as a list, add [1], and improve wording along the way. [1]: #7995 Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>

Despite its documented limitations, the benefits outweight the drawbacks of making this the default analyzer for Gradle projects. Adjust the Pub package manager as well to use `GradleInspector` for its tests, which highlights the additional metadata collected. Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>

sschuberth · 2024-09-11T11:57:10Z

Sorry @fviernau, I had do adjust a few commit messages, but I'm done now!

sschuberth requested review from a team as code owners September 3, 2024 10:20

sschuberth force-pushed the default-gi branch from a5d8d82 to bffba4d Compare September 3, 2024 10:34

fviernau requested changes Sep 3, 2024

View reviewed changes

sschuberth force-pushed the default-gi branch from bffba4d to ed73800 Compare September 3, 2024 11:13

fviernau previously approved these changes Sep 3, 2024

View reviewed changes

sschuberth dismissed fviernau’s stale review via f292f6b September 4, 2024 13:28

sschuberth force-pushed the default-gi branch from ed73800 to f292f6b Compare September 4, 2024 13:28

sschuberth marked this pull request as draft September 4, 2024 15:11

sschuberth force-pushed the default-gi branch 3 times, most recently from c4f6ca5 to 0a49445 Compare September 10, 2024 12:20

sschuberth force-pushed the default-gi branch from 0a49445 to a1dea2a Compare September 11, 2024 11:36

sschuberth marked this pull request as ready for review September 11, 2024 11:37

sschuberth requested review from fviernau and a team September 11, 2024 11:37

fviernau previously approved these changes Sep 11, 2024

View reviewed changes

sschuberth added 2 commits September 11, 2024 13:51

docs(gradle-inspector): Fix the link to the init script resource

cd22c9e

This is a fixup for 7373195. Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>

sschuberth dismissed fviernau’s stale review via 0c0d6b3 September 11, 2024 11:52

sschuberth force-pushed the default-gi branch from a1dea2a to 0c0d6b3 Compare September 11, 2024 11:52

sschuberth requested a review from fviernau September 11, 2024 11:52

sschuberth added 2 commits September 11, 2024 13:53

docs(gradle-inspector): Update the list of known limitations

500e680

Format limitations as a list, add [1], and improve wording along the way. [1]: #7995 Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>

sschuberth force-pushed the default-gi branch from 0c0d6b3 to b9657ff Compare September 11, 2024 11:54

sschuberth force-pushed the default-gi branch from b9657ff to c9db5a7 Compare September 11, 2024 11:56

sschuberth enabled auto-merge (rebase) September 11, 2024 11:57

sschuberth requested a review from a team September 11, 2024 11:57

fviernau approved these changes Sep 11, 2024

View reviewed changes

sschuberth merged commit 1621941 into main Sep 11, 2024
21 of 22 checks passed

sschuberth deleted the default-gi branch September 11, 2024 14:38

sschuberth mentioned this pull request Oct 15, 2024

LocationAwareException when using GradleInspector with Gradle version < 6.8 #9282

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make GradleInspector the new default analyzer for Gradle projects #9070

Make GradleInspector the new default analyzer for Gradle projects #9070

sschuberth commented Sep 3, 2024

codecov bot commented Sep 3, 2024 •

edited

Loading

fviernau Sep 3, 2024

sschuberth Sep 3, 2024

fviernau Sep 3, 2024

fviernau Sep 3, 2024

sschuberth Sep 3, 2024

fviernau Sep 3, 2024

sschuberth commented Sep 10, 2024

sschuberth commented Sep 10, 2024

fviernau left a comment

sschuberth commented Sep 11, 2024

Make GradleInspector the new default analyzer for Gradle projects #9070

Make GradleInspector the new default analyzer for Gradle projects #9070

Conversation

sschuberth commented Sep 3, 2024

codecov bot commented Sep 3, 2024 • edited Loading

Codecov Report

fviernau Sep 3, 2024

Choose a reason for hiding this comment

sschuberth Sep 3, 2024

Choose a reason for hiding this comment

fviernau Sep 3, 2024

Choose a reason for hiding this comment

fviernau Sep 3, 2024

Choose a reason for hiding this comment

sschuberth Sep 3, 2024

Choose a reason for hiding this comment

fviernau Sep 3, 2024

Choose a reason for hiding this comment

sschuberth commented Sep 10, 2024

sschuberth commented Sep 10, 2024

fviernau left a comment

Choose a reason for hiding this comment

sschuberth commented Sep 11, 2024

codecov bot commented Sep 3, 2024 •

edited

Loading