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

Add support for shields.io-based badges #28585

Closed
wants to merge 1 commit into from
Closed

Add support for shields.io-based badges #28585

wants to merge 1 commit into from

Conversation

algernon
Copy link
Contributor

@algernon algernon commented Dec 22, 2023

Adds a new /{username}/{repo}/badges family of routes, which redirect to various shields.io badges. The goal is to not reimplement badge generation, and delegate it to shields.io (or a similar service), which are already used by many. This way, we get all the goodies that come with it: different styles, colors, logos, you name it.

So these routes are just thin wrappers around shields.io that make it easier to display the information we want. The URL is configurable via app.ini, and is templatable, allowing to use alternative badge generator services with slightly different URL patterns.

Fixes #5633 and #23688.

Work sponsored by Codeberg e.V.


This is an alternative solution for #23688 (and #5633), doing things in a very different way than #28102.

Things left to do

  • Make the feature toggleable in app.ini
  • Make the generator URL templateable
  • Error handling:
    • Internal errors return 500
    • Simply not finding the appropriate info (because no releases, or repo units being disabled) redirects to a badge with "not found" in the text section, and a crimson background.
  • Tests
    • Positive tests (things work)
    • Negative tests (things do not work)
  • Documentation

Sample screenshot

Screenshot from 2023-12-22 11-43-16

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 22, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 22, 2023
@lng2020
Copy link
Member

lng2020 commented Dec 22, 2023

I'm neutral to a more general badge implementation. If this PR is accepted, I will close my PR.

@6543
Copy link
Member

6543 commented Dec 22, 2023

It should be able to be at lest deactivatable as awhole, because of dataprotection roules

@algernon
Copy link
Contributor Author

It should be able to be at lest deactivatable as awhole, because of dataprotection roules

Can do, no problem.

@delvh
Copy link
Member

delvh commented Dec 22, 2023

Or even better: We let the user configure the badge generation URL (default: https://shields.io), and if it is blank, it is deactivated.

@delvh
Copy link
Member

delvh commented Dec 22, 2023

But yeah, sounds like a good idea overall.

@algernon
Copy link
Contributor Author

algernon commented Dec 22, 2023

Or even better: We let the user configure the badge generation URL (default: https://shields.io), and if it is blank, it is deactivated.

The URL is already configurable in this PR, defaulting to img.shields.io. I think I'd prefer an explicit ENABLED=false for disabling.

@jolheiser
Copy link
Member

jolheiser commented Dec 22, 2023

TIL you can host your own shields.io 😁

I wonder if it would make sense to have a config URL with placeholders for {label} {text} and {color}?
In practice I'm not sure if that's premature customization or not.

In this case the default setting could be https://shields.io/badge/{label}-{text}-{color} but would allow someone to mix it however they wanted to.

I just looked up an alternative as an example, https://badgen.net/static/{label}/{text}/{color}

@algernon
Copy link
Contributor Author

TIL you can host your own shields.io 😁

I wonder if it would make sense to have a config URL with placeholders for {label} {text} and {color}? In practice I'm not sure if that's premature customization or not.

Yeah, that should be doable. There's existing code that does templated URLs (for ROOT_URL, for example), I suppose I can use a similar mechanism for this URL too.

@algernon
Copy link
Contributor Author

I'm pondering about how to make the badges easy to use in the README... As things stand, you need a full URL, like https://my-forge.example.com/user/repo/badges/release.svg. I can't just adjust the renderer to treat badges/release.svg-like relative URLs specially, because if a repo does have a badges/release.svg file, that would not be possible to reference then.

On the other hand, a large part of the badges use case is when they're displayed elsewhere. In that case, we need the full URL anyway. So I guess that'll be the way.

@algernon algernon marked this pull request as ready for review December 22, 2023 20:38
@algernon
Copy link
Contributor Author

algernon commented Dec 22, 2023

Cleaned up the code, made it possible to disable the feature, and change the badge generator URL template, added a bunch of tests (both positive and negative ones), and an example to app.example.ini, and some docs.

I think this is now ready for review.

models/actions/run.go Outdated Show resolved Hide resolved
@wxiaoguang
Copy link
Contributor

TBH, I have strong preference to #28102 if the concerns there could be resolved.

Adds a new `/{username}/{repo}/badges` family of routes, which redirect
to various shields.io badges. The goal is to not reimplement badge
generation, and delegate it to shields.io (or a similar service), which
are already used by many. This way, we get all the goodies that come
with it: different styles, colors, logos, you name it.

So these routes are just thin wrappers around shields.io that make it
easier to display the information we want. The URL is configurable via
`app.ini`, and is templatable, allowing to use alternative badge
generator services with slightly different URL patterns.

Additionally, for compatibility with GitHub, there's an
`/{username}/{repo}/actions/workflows/{workflow_file}/badge.svg` route
that works much the same way as on GitHub. Change the hostname in the
URL, and done.

Fixes #5633 and #23688.

Work sponsored by Codeberg e.V.

Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
@algernon
Copy link
Contributor Author

Force-pushed an update, changing the route of the action badges: it's now /badges/workflows/{workflow_file}/badge.svg with optional branch and event query parameters. Additionally, for closer GitHub compatibility, the same handler is bound to /actions/workflows/{workflow_file}/badge.svg. (Both under /{username}/{repo}/, of course)

@CanisHelix
Copy link

As of approximately 2 weeks ago shields.io supports release/language badges for Gitea/Forgejo out of the box. More endpoints will be added soon, most likely next will be issues/stars once I get to it. Not sure if this information helps this PR or not but just wanted to share this.

@algernon
Copy link
Contributor Author

algernon commented Jan 3, 2024

As of approximately 2 weeks ago shields.io supports release/language badges for Gitea/Forgejo out of the box. More endpoints will be added soon, most likely next will be issues/stars once I get to it. Not sure if this information helps this PR or not but just wanted to share this.

Sweet, that means that a lot of this PR will be unnecessary soon. I suppose only the Actions parts should remain then, because there is no API where the information could be collected from by shields.io to support them natively.

In the longer run, perhaps the better action would be to add those APIs, and have shields.io support Build (Action) badges for Gitea/Forgejo out of the box, too.

@algernon algernon closed this by deleting the head repository Jan 15, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/docs size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Project badges
9 participants