Skip to content

feat: support github enterprise#607

Merged
suzuki-shunsuke merged 5 commits into
suzuki-shunsuke:mainfrom
ponkio-o:feat/support-ghes
Feb 3, 2025
Merged

feat: support github enterprise#607
suzuki-shunsuke merged 5 commits into
suzuki-shunsuke:mainfrom
ponkio-o:feat/support-ghes

Conversation

@ponkio-o

@ponkio-o ponkio-o commented Feb 3, 2025

Copy link
Copy Markdown
Contributor

issue: #602

ponkio-o
ponkio-o commented Feb 3, 2025
View reviewed changes
Comment thread pkg/cli/runner.go
Comment on lines +163 to +165
if flags.GHEBaseURL == "" {
flags.GHEBaseURL = os.Getenv("GITHUB_API_URL")
}

@ponkio-o ponkio-o Feb 3, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The GITHUB_API_URL has https://<hostname>/api/v3 style, but auto injected /api/v3 suffix when without URL path (and trailing slash) by WithEnterpriseURLs() method.

So, users can use both style:

suzuki-shunsuke
suzuki-shunsuke reviewed Feb 3, 2025
View reviewed changes
Comment thread pkg/github/github.go Outdated
@suzuki-shunsuke

suzuki-shunsuke commented Feb 3, 2025

Copy link
Copy Markdown
Owner

Thank you for your contribution!

@suzuki-shunsuke

suzuki-shunsuke commented Feb 3, 2025

Copy link
Copy Markdown
Owner

Did you confirm that this works well?
I don't use GHES, so I can't confirm it.

@suzuki-shunsuke suzuki-shunsuke added the enhancement New feature or request label Feb 3, 2025
@ponkio-o

ponkio-o commented Feb 3, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Did you confirm that this works well?

Yes. I confirmed working correctly with below commands on our GHES.

$ export GITHUB_TOKEN=$(gh auth token --hostname <GHES_HOST>
$ go run ./cmd/mkghtag/main.go -owner <OWNER> -repo <REPO> -sha d8899dbcc664831276a2b3b6451b75110813a477 -ghe-base-url <GHES_HOST> v0.0.1
INFO[0000] creating a reference                          mkghtag_version= program=mkghtag repo_name=<REPO> repo_owner=<OWNER> sha=d8899dbcc664831276a2b3b6451b75110813a477 tag=v0.0.1
INFO[0000] creating a tag                                mkghtag_version= program=mkghtag repo_name=<REPO> repo_owner=<OWNER> sha=d8899dbcc664831276a2b3b6451b75110813a477 tag=v0.0.1

@suzuki-shunsuke

suzuki-shunsuke commented Feb 3, 2025

Copy link
Copy Markdown
Owner

Thank you. Looks good.

@suzuki-shunsuke suzuki-shunsuke merged commit a97287d into suzuki-shunsuke:main Feb 3, 2025
@ponkio-o ponkio-o deleted the feat/support-ghes branch February 3, 2025 08:43
@suzuki-shunsuke

suzuki-shunsuke commented Feb 3, 2025

Copy link
Copy Markdown
Owner

🎉 https://github.com/suzuki-shunsuke/mkghtag/releases/tag/v0.1.5

suzuki-shunsuke added a commit to suzuki-shunsuke/go-pr-diff that referenced this pull request Apr 15, 2026
@suzuki-shunsuke @claude
test: add integration tests for GetDiff and gitFallback
0b8b3fe 
* TestClient_GetDiff_aquaRegistryPR fetches the diff for
  aquaproj/aqua-registry#51977 via the GitHub API. Honors GITHUB_TOKEN
  to avoid rate limits.
* TestClient_gitFallback_mkghtagPR exercises the git shell-out path
  against suzuki-shunsuke/mkghtag#607 (chosen for its tiny repo size to
  keep the dangling-objects pollution of the local .git negligible).
* Both tests skip in -short mode and when git is not on PATH.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
suzuki-shunsuke added a commit to suzuki-shunsuke/go-pr-diff that referenced this pull request Apr 15, 2026
@suzuki-shunsuke @claude
test: assert full diff against fixture files for mkghtag PR
03cf7b1 
* TestClient_GetDiff_mkghtagPR (renamed from _aquaRegistryPR) now uses
  the same PR (suzuki-shunsuke/mkghtag#607) as the gitFallback test.
* Both integration tests load a fixture from testdata/ and compare the
  full diff with cmp.Diff instead of substring matching.
* Two fixtures are kept because the GitHub API and local `git diff`
  produce different abbreviation lengths in `index <hash>..<hash>`
  lines (GitHub uses the upstream repo's auto-abbrev; local git uses
  the host repo's). Diff bodies are otherwise identical.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
suzuki-shunsuke added a commit to suzuki-shunsuke/go-pr-diff that referenced this pull request Apr 15, 2026
@suzuki-shunsuke @claude
Implement basic function (#2)
9b9834f 
* feat: add prdiff package

* feat: implement GetDiff with GitHub API and git fallback

GetDiff now fetches the PR diff via the GitHub API (PullRequestsService.GetRaw),
and falls back to local git commands when the API call fails. The fallback
resolves base/head/merge-base via the API, runs shallow fetches, and runs
`git diff base...head`, creating a temporary git repo if the cwd is not one.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: add unit tests for Client.GetDiff

Covers the GitHub API success path and the API-failure + git-fallback-failure
path using fakes injected via the existing prsService / reposService
interfaces. Adds go-cmp as a test dependency.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: simplify gitFallback for shallow fetches and head-fork deletion

* Drop the explicit base SHA fetch; the diff GitHub shows for a PR is
  merge_base..head, so only those two commits need to be fetched.
* Switch from `git diff base...head` (3-dot) to `git diff <merge_base>
  <head>` (2-dot). The 3-dot form invokes `git merge-base`, which fails
  with "no merge base" against the shallow fetches we make.
* Always fetch through the base repo URL. GitHub's fork network keeps
  the head SHA reachable from the base repo, so the fallback still
  works after the head fork is deleted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: add integration tests for GetDiff and gitFallback

* TestClient_GetDiff_aquaRegistryPR fetches the diff for
  aquaproj/aqua-registry#51977 via the GitHub API. Honors GITHUB_TOKEN
  to avoid rate limits.
* TestClient_gitFallback_mkghtagPR exercises the git shell-out path
  against suzuki-shunsuke/mkghtag#607 (chosen for its tiny repo size to
  keep the dangling-objects pollution of the local .git negligible).
* Both tests skip in -short mode and when git is not on PATH.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: assert full diff against fixture files for mkghtag PR

* TestClient_GetDiff_mkghtagPR (renamed from _aquaRegistryPR) now uses
  the same PR (suzuki-shunsuke/mkghtag#607) as the gitFallback test.
* Both integration tests load a fixture from testdata/ and compare the
  full diff with cmp.Diff instead of substring matching.
* Two fixtures are kept because the GitHub API and local `git diff`
  produce different abbreviation lengths in `index <hash>..<hash>`
  lines (GitHub uses the upstream repo's auto-abbrev; local git uses
  the host repo's). Diff bodies are otherwise identical.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: add ExampleClient_GetDiff

Demonstrates Client.GetDiff usage in godoc by fetching the diff for
suzuki-shunsuke/mkghtag#1080 (a tiny one-line dependency-bump PR chosen
so the full diff fits cleanly in the example output for byte-exact
verification).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: write GoDoc

* chore: configure AI Agents

* chore: configure golangci-lint

* docs: update README

* ci: set up

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@suzuki-shunsuke suzuki-shunsuke Awaiting requested review from suzuki-shunsuke

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ponkio-o @suzuki-shunsuke