Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GITEA] add new gitea service (release/languages) #9781

Merged
merged 13 commits into from Dec 18, 2023

Conversation

CanisHelix
Copy link
Contributor

@CanisHelix CanisHelix commented Dec 4, 2023

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.

Copy link
Contributor

github-actions bot commented Dec 4, 2023

Warnings
⚠️ This PR modified the server but none of its tests.
That's okay so long as it's refactoring existing code.
Messages
📖 ✨ Thanks for your contribution to Shields, @CanisHelix!
📖

Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS against c39884b

@chris48s chris48s added the service-badge Accepted and actionable changes, features, and bugs label Dec 6, 2023
Copy link
Member

@chris48s chris48s left a comment

Choose a reason for hiding this comment

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

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 Show resolved Hide resolved
services/gitea/gitea-base.js Show resolved Hide resolved
services/gitea/gitea-helper.js Outdated Show resolved Hide resolved
services/gitea/gitea-languages-count.service.js Outdated Show resolved Hide resolved
@CanisHelix CanisHelix marked this pull request as draft December 14, 2023 05:29
@CanisHelix CanisHelix marked this pull request as ready for review December 14, 2023 09:24
services/gitea/gitea-languages-count.service.js Outdated Show resolved Hide resolved
services/gitea/gitea-release.service.js Outdated Show resolved Hide resolved
services/gitea/gitea-languages-count.service.js Outdated Show resolved Hide resolved
services/gitea/gitea-release.service.js Outdated Show resolved Hide resolved

let numberOfPages = 1
if (numberOfItems > 0) {
numberOfPages = Math.ceil(numberOfItems / requestLimit)
Copy link
Member

Choose a reason for hiding this comment

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

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_ITEMS is 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

🚀 Updated review app: https://pr-9781-badges-shields.fly.dev

@chris48s
Copy link
Member

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.

@CanisHelix
Copy link
Contributor Author

CanisHelix commented Dec 18, 2023

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.

@chris48s chris48s added this pull request to the merge queue Dec 18, 2023
Merged via the queue into badges:master with commit 8f1f787 Dec 18, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants