Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement a Generic Package Latest Version Finder #11675

Merged
merged 10 commits into from
Feb 25, 2025
Merged

Implement a Generic Package Latest Version Finder #11675

merged 10 commits into from
Feb 25, 2025

Conversation

kbukum1
Copy link
Contributor

@kbukum1 kbukum1 commented Feb 25, 2025 •
edited
Loading

What are you trying to accomplish?

This PR refactors LatestVersionFinder into PackageLatestVersionFinder, making it more generic and reusable across different ecosystems. The primary goal is to move common logic into common, enabling other ecosystems to extend the abstract class without duplicating logic. Additionally, PackageRelease, PackageDetails, and PackageLanguage have been structured in a standard format, allowing Python and other package managers to adopt a unified approach.

What issues does this affect or fix?

  • Enables reuse of PackageLatestVersionFinder across ecosystems.
  • Moves PackageRelease, PackageDetails, and PackageLanguage into common for broader usage.
  • Ensures ecosystem-specific logic remains in fetchers and registries while leveraging the abstract version finder.

Anything you want to highlight for special attention from reviewers?

  • The Python implementation has already been adapted to extend the PackageLatestVersionFinder.
  • Fetchers and Registries remain specific to each ecosystem while benefiting from shared logic.
  • Tests have been updated to reflect the refactoring.

How will you know you've accomplished your goal?

  • PackageLatestVersionFinder is successfully used across multiple ecosystems.
  • Python and other package managers continue functioning correctly after migration.
  • Tests confirm that package metadata retrieval remains accurate and efficient.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@kbukum1
fix sorbet issues
69a5a2d 
fix common spec issues
kbukum1
kbukum1 commented Feb 25, 2025
View reviewed changes
python/lib/dependabot/python/package/package_details.rb
end
end
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Tip: Moved into common/lib/dependabot/package

kbukum1
kbukum1 commented Feb 25, 2025
View reviewed changes
python/lib/dependabot/python/package/package_language.rb
@@ -1,35 +0,0 @@
# typed: strong
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Tip: Moved into common/lib/dependabot/package

kbukum1
kbukum1 commented Feb 25, 2025
View reviewed changes
python/lib/dependabot/python/package/package_release.rb
@@ -1,80 +0,0 @@
# typed: strong
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Tip: Moved into common/lib/dependabot/package

kbukum1
kbukum1 commented Feb 25, 2025
View reviewed changes
python/lib/dependabot/python/update_checker/latest_version_finder.rb
end
def initialize(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Tip: Moved into dependabot/package/package_latest_version_finder, Dependabot::Package::PackageLatestVersionFinder abstract class.

@kbukum1 kbukum1 marked this pull request as ready for review February 25, 2025 03:32
@kbukum1 kbukum1 requested a review from a team as a code owner February 25, 2025 03:32
abdulapopoola
abdulapopoola reviewed Feb 25, 2025
View reviewed changes
common/lib/dependabot/package/package_language.rb
# Represents a single package version
module Dependabot
module Package
class PackageLanguage
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this? I think we already have a Version class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is specifically keep information related to language from fetched for release. libraries if written for different language it will also include that. That's why we are keeping the details. You can check here in the following code. Example python library version can have something like CP37 (CPython 3.7), (Python 3.7) and so on.

Language keep information such as language, version and requirement all together for specific release. Currently we are using requirement to filter out versions.

abdulapopoola
abdulapopoola reviewed Feb 25, 2025
View reviewed changes
common/lib/dependabot/package/package_release.rb
# Represents a single package version
module Dependabot
module Package
class PackageRelease
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe call this Release instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Release so general. So was thinking it may mix up with other things?

abdulapopoola
abdulapopoola approved these changes Feb 25, 2025
View reviewed changes
Copy link
Member

@abdulapopoola abdulapopoola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed that this is a refactoring and move of existing code; ignore my earlier comments.

@kbukum1
Copy link
Contributor Author

kbukum1 commented Feb 25, 2025

Just noticed that this is a refactoring and move of existing code; ignore my earlier comments.

Thanks for the review

@kbukum1 kbukum1 enabled auto-merge (squash) February 25, 2025 21:53
@kbukum1 kbukum1 disabled auto-merge February 25, 2025 21:53
@kbukum1 kbukum1 merged commit d00807a into main Feb 25, 2025
178 of 184 checks passed
@kbukum1 kbukum1 deleted the kamil/generalize_package_latest_version_finder branch February 25, 2025 21:56
dmitris pushed a commit to dmitris/dependabot-core that referenced this pull request Feb 26, 2025
@kbukum1 @dmitris
Implement a generic package latest version finder (dependabot#11675)
6d87782
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abdulapopoola abdulapopoola abdulapopoola approved these changes

@robaiken robaiken Awaiting requested review from robaiken

Assignees
No one assigned
Labels
L: python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kbukum1 @abdulapopoola