Skip to content

Fetch GitHub stars for any GitHub-hosted dist (and fix index_github_bugs no-op) - #1491

Open
oalders wants to merge 6 commits into
masterfrom
fix-1477
Open

Fetch GitHub stars for any GitHub-hosted dist (and fix index_github_bugs no-op)#1491
oalders wants to merge 6 commits into
masterfrom
fix-1477

Conversation

@oalders

@oalders oalders commented Jun 14, 2026

Copy link
Copy Markdown
Member

Closes #1477

Problem

GitHub star/watcher counts were only fetched for distributions whose bug tracker was GitHub issues. A dist that uses RT (or any non-GitHub tracker) but hosts its code on GitHub never had its star count fetched.

While fixing this I found a deeper bug: index_github_bugs has been a silent no-op since November 2024. Commit 7c471c0 migrated the loop from a model scroll ($release->resources) to a raw Elasticsearch scroll_helper but kept top-level hash access ($release->{resources}, $release->{distribution}). Raw ES hits store fields under _source, so both reads returned undef, every release was skipped, and no GitHub bug or star data was being updated at all.

Changes

  • Read release fields from _source in index_github_bugs, matching check_all_distributions and every other script. This restores the feature.
  • Decouple star fetching from bug counting: the release query now also matches resources.repository.url / resources.repository.web, and _github_dist_summary records stars/watchers for any dist with a GitHub repo while recording GitHub issue counts only when GitHub issues are the bug tracker.
  • _is_github_url parses the URL and compares its host to github.com (via URI->new->host) instead of a prefix match, rejecting look-alike hosts such as github.com.evil.com. Adds URI::git (cpanfile + snapshot) so git:// URLs resolve a host.
  • The git:// scheme is matched only against repository URL fields (bug trackers are always http(s)).
  • Skip releases lacking a distribution, and skip a null repository in an otherwise-successful GraphQL response before dereferencing it.

Testing

  • New t/script/tickets.t unit tests for the host check, the query filter, and the star-vs-issue decision logic (including the RT-bugtracker-plus-GitHub-repo case that motivates the fix).
  • prove t/script/tickets.t t/script/load.t passes; precious lint clean (perlimports/perlcritic/perltidy).
  • Red-green verified: simulating the old coupled behavior makes the decoupling tests fail.
  • cpm install --resolver=snapshot URI::git resolves from the new snapshot stanza.
  • Not verified end-to-end: a live index_github_bugs run needs a GitHub token and ES release fixtures, which weren't available here. The _source fix is verified by consistency with the rest of the codebase rather than an integration test.

@codecov

codecov Bot commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.16667% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.28%. Comparing base (a4fd17a) to head (722d9bc).

Files with missing lines Patch % Lines
lib/MetaCPAN/Script/Tickets.pm 79.16% 8 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1491      +/-   ##
==========================================
+ Coverage   63.98%   64.28%   +0.29%     
==========================================
  Files         137      137              
  Lines        4468     4502      +34     
  Branches      671      682      +11     
==========================================
+ Hits         2859     2894      +35     
+ Misses       1352     1349       -3     
- Partials      257      259       +2     
Files with missing lines Coverage Δ
lib/MetaCPAN/Script/Tickets.pm 74.79% <79.16%> (+10.75%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread lib/MetaCPAN/Script/Tickets.pm Outdated
oalders and others added 4 commits September 1, 2026 21:53
GitHub star/watcher counts were only fetched for distributions whose bug
tracker was GitHub issues. Distributions that use RT (or another tracker)
but host their code on GitHub never had their star counts fetched.

Broaden the release query to also match resources.repository.url/web, and
decouple star fetching from bug counting: stars/watchers are now recorded
for any dist with a GitHub repository, while GitHub issue counts remain
tied to GitHub being the bug tracker.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- _is_github_url now extracts the host via URI::Split and compares it to
  github.com, instead of a prefix match. This rejects look-alike hosts
  (github.com.evil.com) and is the requested URI-based parsing.
- Guard against a null repository in an otherwise-successful GraphQL
  response before dereferencing it.
- Restrict the git:// scheme to repository URL fields in the release
  query (bug trackers are always http(s)).
- Expand tests for the above; format with precious.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two fixes:

1. Read release fields from _source. Commit 7c471c0 migrated
   index_github_bugs from a model scroll (\$release->resources) to a raw
   Elasticsearch scroll but kept top-level hash access (\$release->{resources},
   \$release->{distribution}). Raw ES hits store fields under _source, so both
   reads returned undef, github_user_repo_from_resources got undef, and every
   release was skipped -- the script has fetched no GitHub data since that
   commit. Read from \$release->{_source} like the rest of the codebase.

2. _is_github_url now parses the host with URI->new->host and compares it to
   github.com. URI::git is added (cpanfile + snapshot) so git:// URLs parse to
   a host instead of a host-less foreign URI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mirrors the guard in check_all_distributions and avoids a wasted GitHub
API call (and a stray $summary{undef} entry) for the unexpected case of a
latest release with no distribution.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The recursive fallback scan returned the first github url it found while
iterating values in hash order, making the (user, repo) result
non-deterministic. Sort keys so the match is consistent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N6oJpF2cBxDcSPevtz6n6Z
@oalders
oalders marked this pull request as ready for review September 1, 2026 21:55
precious 0.12 rejects the singular `label` field (expects `labels`), so
CI could not load precious.toml at all and Code Formatting failed. A
labeled command only runs under an explicit --label, but CI runs bare
`precious lint`, so renaming to `labels` would have dropped perltidy from
the default run. Remove the label instead so perltidy runs by default
alongside perlimports and perlcritic, keeping formatting enforced in CI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N6oJpF2cBxDcSPevtz6n6Z
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

github star counts only fetched if github issues used

2 participants