feat: Gitea pull request label support#6101
feat: Gitea pull request label support#6101gnanirahulnutakki wants to merge 2 commits intoakuity:mainfrom
Conversation
Signed-off-by: Gnani Rahul <gnutakki@radiantlogic.com>
✅ Deploy Preview for docs-kargo-io ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
gnanirahulnutakki
changed the title
| repoLabels, _, err := p.client.ListRepoLabels( | ||
| p.owner, | ||
| p.repo, | ||
| gitea.ListLabelsOptions{}, | ||
| ) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error listing repository labels: %w", err) | ||
| } |
There was a problem hiding this comment.
ListRepoLabels is called with an empty gitea.ListLabelsOptions{}, which uses Gitea's default page size https://gitea.com/gitea/go-sdk/src/tag/gitea/v0.20.0/gitea/list_options.go#L19.
If a repository has more labels than the default, the function will not see them and will incorrectly report them as missing.
One solution for making sure we are listing all labels here could be incrementing opts.Page in a loop until the response is empty.
I'd like to see a test for this too.
There was a problem hiding this comment.
Paged label lookup now walks ListRepoLabels using Response.NextPage until exhaustion instead of assuming the first page contains every repository label. I added regression coverage for a label that only appears on a later page so this does not silently regress.
| if len(labelIDs) > 0 { | ||
| if _, _, err := p.client.AddIssueLabels( | ||
| p.owner, | ||
| p.repo, | ||
| giteaPR.Index, | ||
| gitea.IssueLabelsOption{Labels: labelIDs}, | ||
| ); err != nil { | ||
| return nil, fmt.Errorf( | ||
| "error adding labels to pull request %d: %w", | ||
| giteaPR.Index, | ||
| err, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
So if we get here it means we successfully created a PR but because we failed to add labels we report an error.
We could improve the error here e.g. successfully created pr number %d but failed to add labels: %w.
Although, I think a better solution would be to not have this be a separate call at all.
It looks like CreatePullRequestOption has a Labels field: https://gitea.com/gitea/go-sdk/src/tag/gitea/v0.20.0/gitea/pull.go#L159 and since you already resolve labelIDs before the CreatePullRequest call it makes more sense to just add your labelIDs there.
There was a problem hiding this comment.
The separate AddIssueLabels call is gone. Resolved label IDs are now passed directly in CreatePullRequestOption.Labels, so label application stays part of PR creation instead of becoming a second failure point after the PR already exists. The updated test asserts that path, and I also validated it end-to-end against a live OrbStack-backed Gitea instance with a real paginated label set.
fuskovic
added
area/controller
Signed-off-by: Gnani Rahul <gnutakki@radiantlogic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6101 +/- ##
==========================================
+ Coverage 57.44% 57.47% +0.03%
==========================================
Files 474 474
Lines 40230 40277 +47
==========================================
+ Hits 23110 23151 +41
- Misses 15745 15748 +3
- Partials 1375 1378 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fuskovic
changed the title
| labelIDsByName := make(map[string]int64, len(repoLabels)) | ||
| for _, repoLabel := range repoLabels { | ||
| if repoLabel == nil { | ||
| continue | ||
| } | ||
| labelIDsByName[repoLabel.Name] = repoLabel.ID | ||
| } |
There was a problem hiding this comment.
This loop is unnecessary — you can build the map directly inside the pagination loop, eliminating the intermediate repoLabels slice entirely.
3 participants
Fixes #6021
Summary
Testing
go test ./pkg/gitprovider/gitea./hack/bin/golangci-lint run --config .golangci.yaml ./pkg/gitprovider/gitea/...