[GITEA] add new gitea service (release/languages) #9781
Conversation
|
There was a problem hiding this comment.
Hi thanks for working on this.
I've done a first pass of review. In general the implementations look pretty sensible. We seem to be sticking pretty close to GitLab as a source of inspiration. I also agree that starting off with just a couple of badges and then iterating from there to add more in follow-up PRs is an ideal way to approach this 👍
I've done a first pass of review and left a few comments. This one might take a few iterations but this is off to a good start.
services/gitea/gitea-base.js
Outdated
|
|
||
| let numberOfPages = 1 | ||
| if (numberOfItems > 0) { | ||
| numberOfPages = Math.ceil(numberOfItems / requestLimit) |
There was a problem hiding this comment.
Hmm. I have not had a really detailed look at this pagination code, but I'm not sure this is going to work.
If we look at
https://docs.gitea.com/next/development/api-usage
(which I think is relevant)
MAX_RESPONSE_ITEMSis set to 50
says to me that
a) The default max allowable value of requestLimit for a gitea server is 50
b) This is a number that is going to be different for different gitea servers, so we can't safely assume its value
so I think trying to assume we can work out the number of pages up-front based on our hard-coded value of requestLimit and the total number of items feels like it is logic based on a bad assumption.
Tbh, the way we do this for the GitHub badges is we just request the first page of releases ordered by date, and if you want the latest semver then the 'search space' for that is the first page of results, mostly for performance reasons. We've yet to have anyone pop up whose latest release by semver isn't in the first page of results.
Tbh, I'd be happy with that solution here too. When I flagged that fetchPage was unused code, I assumed we'd probably just delete it.
Given we have to deal with multiple different implementations, if we do want to try and implement pagination like this, we'd need to infer the page size rather than assuming it up-front, but as I say, I'd also be happy to just gloss over it if we can.
There was a problem hiding this comment.
We have MAX_RESPONSE_ITEMS for the absolute maximum, and DEFAULT_PAGING_NUM for the default pagination value which can be different per every instance. We do get a header for x-total-count for how many items are available, and if this is greater than the number of items in the array we could calculate the pages.
When I found the pagination was not working in Gitlab's code too I thought to myself... who even has more than 100 releases to even scan through to find the latest SemVer! Happy to put this back and just assume the first page too.
|
🚀 Updated review app: https://pr-9781-badges-shields.fly.dev |
|
Cool. I've done a bit of testing. Latest changes LGTM. One final detail. For both badges, the example we're using is https://codeberg.org/go-gitea/gitea but that isn't an actual project. Can we pick a repo that exists for the example please. |
|
I've updated the examples to point to https://codeberg.org/forgejo/forgejo/ as that's a guaranteed stable repository at codeberg.org. If ever Gitea do a canonical host we can update theirs later. |
| example: 'release', | ||
| }), | ||
| queryParam({ | ||
| name: 'date_order_by', |
There was a problem hiding this comment.
@CanisHelix late to the game on this one, but I happened to stumble upon this file. It doesn't look this query parameter or the corresponding orderBy parameter lower down is used, am I missing something?
This adds an initial Gitea Service with badges for Releases and Language counts. https://codeberg.org/ (A Gitea Compliant website, albeit using a fork of Gitea) is used for most tests with a dedicated repository for testing again for controlled tests
Private Repository support added and the service document updated.
Uses the new openAPI method instead of old examples.
This would contribute towards issue: #4040, with the aim to add more badges/endpoints overtime.