Fix version sorting by using SemVer instead of publishedDate#168
Fix version sorting by using SemVer instead of publishedDate#168Moustachauve merged 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the retrieval of the latest stable and beta versions by replacing specific SQL queries with a general repository fetch followed by client-side filtering and sorting using a new semantic versioning comparator. Feedback focuses on improving performance and memory efficiency by fetching version metadata before loading assets and optimizing the comparator to avoid redundant parsing of version tags.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request migrates the logic for identifying the latest stable and beta versions from SQL queries in the DAO to the repository layer. It introduces a SemVerComparator using the semver4j library to ensure versions are compared correctly, with a fallback to the publication date for invalid semantic tags. Unit tests have been added to validate this new sorting behavior. The review feedback suggests improving code readability and idiomaticity by using imports instead of fully qualified names, adopting camelCase for property names, and utilizing when expressions.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request migrates the logic for identifying the latest stable and beta versions from SQL queries in VersionDao to Kotlin code within VersionWithAssetsRepository. It introduces a semVerComparator that uses the Semver library to accurately compare version tags, falling back to publication dates when necessary. Unit tests were added to verify the sorting logic. The reviewer suggested changing the visibility of the new comparator and helper function to internal to maintain a cleaner public API.
| versionDao.getVersionByTagName(repositoryId, tagName) | ||
|
|
||
| companion object { | ||
| val semVerComparator = |
There was a problem hiding this comment.
The semVerComparator is currently public. Since it is an implementation detail used for sorting versions within this repository and its associated tests, consider changing its visibility to internal to maintain a cleaner public API.
| val semVerComparator = | |
| internal val semVerComparator = |
| } | ||
| } | ||
|
|
||
| fun getLatestVersion(versions: List<Version>): Version? = versions.map { |
There was a problem hiding this comment.
The getLatestVersion function is currently public. Consider making it internal to restrict its scope to the module, as it is primarily a helper for the repository's public methods and for unit testing.
| fun getLatestVersion(versions: List<Version>): Version? = versions.map { | |
| internal fun getLatestVersion(versions: List<Version>): Version? = versions.map { |
Fixes #167