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

Restore the blacklisters-approved/rejected icons in PR comments #11283

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

Spevacus
Copy link
Contributor

@Spevacus Spevacus commented May 23, 2024

We used to store these icons behind a camo.githubusercontent.com link, but attempting to access that link throws a 403 now.

Example PR comment where the image is no longer visible: #11282 (comment)

The source for these images is from https://shields.io/ 's static badge generator. It has an API that this PR uses to generate and return a specified label with a given style.

We just need a simple green/red coloring for the "blacklisters|approved" and "blacklisters|rejected" actions we perform, and the URL for that is very simple.

Here's how these two URLs will be rendered in a comment:

Approved with SmokeyApprove

Where the markdown is ![Approved with SmokeyApprove](https://img.shields.io/badge/blacklisters-approved-green)

Rejected with SmokeyReject

Where the markdown is ![Rejected with SmokeyReject](https://img.shields.io/badge/blacklisters-rejected-red)

Shields.io has been around for a loooong time, since 2013 (?) and is unlikely to be deprecated in the near term as far as I can tell. An alternative approach, which we could do, is to store the image in SE's i.sstatic domain instead and refer to that. I am not against changing this to do so instead of relying upon shields.io's API, if requested.

@tripleee tripleee merged commit 3367682 into Charcoal-SE:master Jun 11, 2024
3 checks passed
@tripleee
Copy link
Member

Thanks for the contribution, and sorry for leaving it unreviewed for so long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants