Cocoapods: Improve the handling for dependencies with checkout option #11100
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
plugins/package-managers/cocoapods/src/main/kotlin/PodDependencyHandler.kt
Fixed
Show fixed
Hide fixed
fviernau
force-pushed
the
cocoapods-checkout-options-handling
branch
from
3f28bb6 to
92261fc
Compare
plugins/package-managers/cocoapods/src/main/kotlin/PodDependencyHandler.kt
Outdated
Show resolved
Hide resolved
| val podspec = getPodspec(dependency.name, dependency.version) | ||
|
|
||
| val vcs = podspec?.toVcsInfo().orEmpty() | ||
| val vcs = checkoutOption?.toVcsInfo() ?: podspec?.toVcsInfo().orEmpty() |
There was a problem hiding this comment.
gets additional data, such as declared licenses and the source artifact url.
I would have expected adjustments to expected test results because of that, but I guess there's no coverage. Can you confirm this?
There was a problem hiding this comment.
Let's just wait for the tests to finish, for the confirmation.
I have not looked into enhancing the tests.
From reading the code a couple of days before, the handling raised question marks which I tried to address here.
There was a problem hiding this comment.
and gets additional data
I still feel a bit uneasy about that statement, as we cannot prove it via enriched expected results. While theoretically it should be possible to get additional data this way, I would feel better if we could soften the wording here a bit and say e.g. "and might get additional data".
| val revision = commit ?: tag ?: branch ?: return null | ||
| val url = git ?: return null |
There was a problem hiding this comment.
Also here, just to note that this does not seem to improve any current test data.
fviernau
force-pushed
the
cocoapods-checkout-options-handling
branch
from
92261fc to
52b3bfe
Compare
fviernau
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11100 +/- ##
=========================================
Coverage 57.38% 57.38%
Complexity 1703 1703
=========================================
Files 346 346
Lines 12825 12825
Branches 1214 1214
=========================================
Hits 7360 7360
Misses 4997 4997
Partials 468 468
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Improve readability and prepare for an upcoming change. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>
Prepare for an upcoming change. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>
Prepare for an upcoming change. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>
fviernau
force-pushed
the
cocoapods-checkout-options-handling
branch
from
52b3bfe to
c0013c0
Compare
| val podspec = getPodspec(dependency.name, dependency.version) | ||
|
|
||
| val vcs = podspec?.toVcsInfo().orEmpty() | ||
| val vcs = checkoutOption?.toVcsInfo() ?: podspec?.toVcsInfo().orEmpty() |
There was a problem hiding this comment.
and gets additional data
I still feel a bit uneasy about that statement, as we cannot prove it via enriched expected results. While theoretically it should be possible to get additional data this way, I would feel better if we could soften the wording here a bit and say e.g. "and might get additional data".
The information of podspecs corresponding to dependencies which have a checkout option has been ignored so far. Unify the code paths, so that the information from podspecs is always used. This simplifies the code and might get additional data, such as declared licenses and the source artifact url. Furthermore, the VCS URL from the checkout option is no more used as `homepageUrl`, but instead always set to the dedicated `homepage` field of the podspec. This is more consistent with integration of other package managers. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>
Signed-off-by: Frank Viernau <frank.viernau@gmail.com>
If the VCS info in checkout options is incomplete, fallback to the VCS info from the podspec instead of using a (partially) empty VCS info. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>
fviernau
force-pushed
the
cocoapods-checkout-options-handling
branch
from
c0013c0 to
806377a
Compare
|
Merging despite the unrelated test failure(s). |
Unify code paths to simplify and to obtain additional data.