Skip to content

feat: Add repo download contents sentinel errors#4192

Merged
gmlewis merged 1 commit intogoogle:masterfrom
stevehipwell:add-download-contents-errs
May 7, 2026
Merged

feat: Add repo download contents sentinel errors#4192
gmlewis merged 1 commit intogoogle:masterfrom
stevehipwell:add-download-contents-errs

Conversation

@stevehipwell
Copy link
Copy Markdown
Contributor

This PR adds Sentinel errors to the RepositoriesService.DownloadContents & RepositoriesService.DownloadContentsWithMeta functions. This is a follow-up to #4153 that improves the ergonomics. These sentinels are required to correctly handle the different paths in these functions without falling back to writing logic against the error messages which is brittle (and are changing in the next release).

@gmlewis since working on #4153 I've found more issues with the pattern it replaced such as always returning file as the type for a submodule due to the directory loop pattern and a "bug" in the 2022 API version. This combined with a lack of Sentinel errors makes using the current released version painful, would it be possible to get a release out once this PR is merged?

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
@gmlewis gmlewis changed the title feat: add repo download contents sentinel errs feat: Add repo download contents sentinel errors May 7, 2026
@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label May 7, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.71%. Comparing base (6c643b8) to head (9cf990f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4192   +/-   ##
=======================================
  Coverage   93.71%   93.71%           
=======================================
  Files         209      209           
  Lines       19770    19772    +2     
=======================================
+ Hits        18527    18529    +2     
  Misses       1046     1046           
  Partials      197      197           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @stevehipwell!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @alexandear - @zyfy29 - @Not-Dhananjay-Mishra - @munlicode

Copy link
Copy Markdown
Contributor

@Not-Dhananjay-Mishra Not-Dhananjay-Mishra left a comment

Choose a reason for hiding this comment

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

LGTM

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label May 7, 2026
@gmlewis
Copy link
Copy Markdown
Collaborator

gmlewis commented May 7, 2026

Thank you, @Not-Dhananjay-Mishra!
Merging and I will follow up with a new release shortly thereafter.

@gmlewis gmlewis merged commit 9c0ad62 into google:master May 7, 2026
10 checks passed
@stevehipwell stevehipwell deleted the add-download-contents-errs branch May 7, 2026 17:02
@gmlewis
Copy link
Copy Markdown
Collaborator

gmlewis commented May 8, 2026

@gmlewis since working on #4153 I've found more issues with the pattern it replaced such as always returning file as the type for a submodule due to the directory loop pattern and a "bug" in the 2022 API version. This combined with a lack of Sentinel errors makes using the current released version painful, would it be possible to get a release out once this PR is merged?

@stevehipwell - this is now released here:
https://github.com/google/go-github/releases/tag/v86.0.0

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