Skip to content

Conversation

@sanchitram1
Copy link
Contributor

  • ruff
  • DRY ingests and centralize setting URLs

@sanchitram1 sanchitram1 requested a review from jhheider June 19, 2025 15:27
@jhheider jhheider requested a review from Copilot June 19, 2025 15:37
Copy link
Member

@jhheider jhheider left a comment

Choose a reason for hiding this comment

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

DRY always good.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes duplicated ingest logic from pkgx and Homebrew modules, centralizes URL-loading routines in core/db.py, and aligns function signatures and test code accordingly.

  • Extracts URL building into _build_current_urls with current_urls and all_current_urls helpers in core/db.py
  • Removes bespoke ingest and manual URL-fetch implementations in package_managers/pkgx/db.py and package_managers/homebrew/db.py
  • Updates pipeline calls in pkgx/main.py and homebrew/main.py to match reordered parameters
  • Cleans up a test in test_pkgx_diff.py to use next(iter(...)) for key retrieval

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/package_managers/pkgx/test_pkgx_diff.py Switched from list(keys)[0] to next(iter(keys))
package_managers/pkgx/main.py Reordered run_pipeline arguments
package_managers/pkgx/db.py Removed custom set_current_urls and ingest, now calls core helper
package_managers/homebrew/main.py Reordered main arguments
package_managers/homebrew/db.py Removed custom ingest, delegates URL-loading to core
core/db.py Added _build_current_urls, current_urls, and all_current_urls

@sanchitram1 sanchitram1 merged commit f0d3de7 into main Jun 19, 2025
1 check passed
@sanchitram1 sanchitram1 deleted the DRY-ingests branch June 19, 2025 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants