Repository navigation
refactor(forge): share one REST client between GitHub, Gitea and GitLab pagination - #1287
Conversation
There was a problem hiding this comment.
Behaviour matches the old per-forge code: same headers (GitHub comment calls now also send X-GitHub-Api-Version, which is harmless), same page sizes, same error contexts and codes. Switching to .query() for paging is an improvement. Test and Coverage were still running when I reviewed.
Nit: RestClient::gitlab (src/forge/gitlab.rs:54) is only used for pagination. Its repo_url is built and then never read, and the client still exposes create_release, create_comment, update_comment and find_comment. Those methods use GitHub/Gitea paths and payloads (/issues/..., body rather than description), so a later caller would get a client that looks valid and hits the wrong GitLab endpoints. I'd move the pagination loop into a free paginate(request_fn, ...) that all three forges share, and keep RestClient for GitHub and Gitea only. If you'd rather keep the GitLab constructor, at least add a doc comment saying only paginated_json_array applies.
Closes #1285
gitea.rswas 36.7% duplicated fromgithub.rs(Sonar): pagination, the three release calls and the three comment calls, identical except for headers, the page-size parameter and the error codes. GitLab had a third copy of the pagination loop.src/forge/rest.rsnow holds aRestClientbuilt per forge with its auth header, extra headers and page-size parameter (RestClient::github,::gitea,::gitlab). It owns pagination,create_release,find_draft_release,publish_release,find_comment,create_commentandupdate_comment. Each forge keeps its own context messages and error codes by wrapping the call, so the error chain users see is unchanged.Two small differences worth knowing:
query, giving the same?per_page=…&page=…(or?limit=…on Gitea) URL as before.X-GitHub-Api-Version, which the release and pagination calls already did. That is the documented header; the inconsistency was accidental.percent_encode_pathmoves fromconfig::loader_jsto a neutralcrate::urimodule, since both forges and the JS loader use it (raised in the reviews of #1267 and #1269).Forge and config tests pass (518), including the fake-server API tests for each forge.