Skip to content

refactor: Abstract general behaviour from TopPypiReference#299

Merged
sdn4z merged 1 commit into
elementsinteractive:mainfrom
sdn4z:abstract-pkg-ref
Sep 3, 2025
Merged

refactor: Abstract general behaviour from TopPypiReference#299
sdn4z merged 1 commit into
elementsinteractive:mainfrom
sdn4z:abstract-pkg-ref

Conversation

@sdn4z
Copy link
Copy Markdown
Collaborator

@sdn4z sdn4z commented Sep 2, 2025

This is the first step towards #96
In this PR we abstract all the general logic that will be shared across sources.

@scastlara
Copy link
Copy Markdown
Collaborator

/lgtm review

Comment thread src/twyn/trusted_packages/references.py
@github-actions github-actions Bot added refactor and removed refactor labels Sep 3, 2025
@scastlara
Copy link
Copy Markdown
Collaborator

/lgtm review

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

🦉 lgtm Review

Score: Needs Work 🔧

🔍 Summary

This PR is a good step towards abstracting the logic for different package sources. The new structure with AbstractPackageReference and a concrete TopPyPiReference is well-designed.

However, there are a couple of critical correctness issues in the implementation of AbstractPackageReference that will cause runtime errors:

  1. The class is not correctly defined as an Abstract Base Class (it's missing inheritance from ABC).
  2. The _parse method is not marked as abstract and is defined as a static method, leading to an incorrect implementation that will break the get_packages logic.

I have also pointed out a couple of minor typos in the new docstrings.

After addressing these points, the PR should be in great shape.

More information
  • Id: dacae0e23b184a37bee76c4dc53ebfab
  • Model: gemini-2.5-pro
  • Created at: 2025-09-03T08:46:54.583419+00:00
Usage summary
  • Request count: 2
  • Request tokens: 28,495
  • Response tokens: 17,762
  • Total tokens: 46,257

See the 📚 lgtm-ai repository for more information about lgtm.

Comment thread src/twyn/trusted_packages/references.py
Comment thread src/twyn/trusted_packages/references.py
Comment thread src/twyn/trusted_packages/references.py
Comment thread src/twyn/trusted_packages/references.py
@github-actions github-actions Bot added refactor and removed refactor labels Sep 3, 2025
@sdn4z sdn4z enabled auto-merge (squash) September 3, 2025 11:28
@sdn4z sdn4z merged commit b3f7d07 into elementsinteractive:main Sep 3, 2025
12 checks passed
@sdn4z sdn4z deleted the abstract-pkg-ref branch September 10, 2025 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants