Refactor Pub's pubspec parsing to use a data class and KxS / kaml #9080
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9080 +/- ##
============================================
+ Coverage 68.30% 68.32% +0.01%
Complexity 1153 1153
============================================
Files 237 237
Lines 7576 7576
Branches 865 865
============================================
+ Hits 5175 5176 +1
+ Misses 2048 2047 -1
Partials 353 353
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
fviernau
force-pushed
the
pub-podspec-kxs
branch
from
8774fd9 to
05a2e64
Compare
| ) : Dependency | ||
| } | ||
|
|
||
| private class DependencyMapSerializer : KSerializer<Map<String, Dependency>> { |
There was a problem hiding this comment.
Can this inherit from StdDeserializer instead to not have the "dummy override" for serialize (and to avoid the strict need for the descriptor override)?
There was a problem hiding this comment.
No, because StdSerializer extends from JsonSerializer, while this one is for YAML.
There was a problem hiding this comment.
Sorry, I mixed up the Jackson's StdSerializer with KxS's JsonTransformingSerializer anyway.
So I guess the missing KAML feature is charleskorn/kaml#29? If so, how about adding serializer class docs with a reference to that issue?
| @@ -750,7 +750,7 @@ class Pub( | |||
| private fun containsFlutterSdk(workingDir: File): Boolean { | |||
| val pubspec = parsePubspec(workingDir.resolve(PUBSPEC_YAML)) | |||
|
|
|||
| return (pubspec.dependencies.orEmpty()["flutter"] as? Pubspec.SdkDependency)?.sdk == "flutter" | |||
| return pubspec.dependencies.orEmpty().values.any { (it as? Pubspec.SdkDependency)?.sdk == "flutter" } | |||
There was a problem hiding this comment.
Commit message:
- "Take" -> "Make"
- Please explain why that is better and safe to do. Maybe @MarcelBochtler can help to find an argument here?
There was a problem hiding this comment.
You can have a dependency on flutter_driver (IIRC), which is part of the flutter SDK.
There was a problem hiding this comment.
In one of our lockfile for example we have:
sky_engine:
dependency: transitive
description: flutter
source: sdk
version: "0.0.99"
There was a problem hiding this comment.
I've split out this commit to #9082.
fviernau
force-pushed
the
pub-podspec-kxs
branch
from
05a2e64 to
fb70131
Compare
fviernau
force-pushed
the
pub-podspec-kxs
branch
2 times, most recently
from
5cccaac to
2a6f370
Compare
|
There's one Detekt issue left, it seems. |
Signed-off-by: Frank Viernau <[email protected]>
Avoid a naming conflict in an upcoming change. Signed-off-by: Frank Viernau <[email protected]>
Align on `pubspec` as the term. Signed-off-by: Frank Viernau <[email protected]>
Signed-off-by: Frank Viernau <[email protected]>
fviernau
force-pushed
the
pub-podspec-kxs
branch
from
2a6f370 to
197d265
Compare
See individual commits.