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 [Raycast] Badge #9801

Merged
merged 5 commits into from Dec 20, 2023
Merged

Add [Raycast] Badge #9801

merged 5 commits into from Dec 20, 2023

Conversation

Fatpandac
Copy link
Contributor

image

Copy link
Contributor

github-actions bot commented Dec 11, 2023

Messages
📖 ✨ Thanks for your contribution to Shields, @Fatpandac!

Generated by 🚫 dangerJS against 5f29a85

@Fatpandac Fatpandac changed the title Add [Raycast extension installs] Badge Add [Raycast] Badge Dec 11, 2023
async fetch({ user, extension }) {
return this._requestJson({
schema,
url: `https://www.raycast.com/frontend_api/users/${user}/extensions/${extension}`,
Copy link
Member

@chris48s chris48s Dec 11, 2023

Choose a reason for hiding this comment

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

Hi. I haven't had a proper look at this code, but this looks like it might be an internal API endpoint, rather than a public one.

Can you link to any documentation for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is not an open API. I found it on the website of Raycast and it does not have any documentation.

Copy link
Member

Choose a reason for hiding this comment

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

OK thanks. The reason we say

Badges should not obtain data from undocumented or reverse-engineered API endpoints.

in our guidelines

is because we had some situations where a contributor worked out that a project had an API by inspecting the frontend code for their website or whatever, added a badge that calls it, and then we ended up relying on an API that wasn't really intended for public use. This then either annoyed the maintainers of the company/project behind it or we had no reasonable expectation of stability/compatibility over time (because devs maintaining the the API were making the assumption the only thing consuming it was their own frontend code) and stuff kept breaking.

That's looks like the situation we've got here so I am going to close this PR, but we could revisit in future if there is a reliable public source for the data. Other than the data source, it looks like a badge showing downloads for a raycast extension would be a reasonable badge to add.

Copy link

@RSO RSO Dec 18, 2023

Choose a reason for hiding this comment

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

👋 I'm a Software Engineer @ Raycast. We don't have public API documentation available, but if you use https://backend.raycast.com/api/v1/extensions/petr/slack-status instead of the frontend_api path, we're good with publishing the badge. We'll make sure the endpoint keeps being supported.

Copy link
Contributor Author

@Fatpandac Fatpandac Dec 18, 2023

Choose a reason for hiding this comment

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

Thanks for your reply 🙏, I will change to use this API soon.
Could you reopen this PR? @chris48s

Copy link
Member

Choose a reason for hiding this comment

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

yeah sounds reasonable - I'll have a proper look over the code once this is updated

@chris48s chris48s closed this Dec 12, 2023
@chris48s chris48s added the service-badge Accepted and actionable changes, features, and bugs label Dec 12, 2023
@chris48s chris48s reopened this Dec 18, 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.

Thanks for updating. I've left some comments. There's quite a few points to follow up but they should all be quite quick and easy to address.

services/raycast/installs.service.js Outdated Show resolved Hide resolved
services/raycast/installs.service.js Outdated Show resolved Hide resolved
services/raycast/installs.service.js Outdated Show resolved Hide resolved
services/raycast/installs.service.js Outdated Show resolved Hide resolved
services/raycast/installs.service.js Outdated Show resolved Hide resolved
services/raycast/installs.service.js Outdated Show resolved Hide resolved
services/raycast/installs.service.js Outdated Show resolved Hide resolved
services/raycast/installs.service.js Outdated Show resolved Hide resolved
services/raycast/installs.tester.js Outdated Show resolved Hide resolved
Copy link
Contributor

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

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.

latest changes lgtm 👍

@chris48s chris48s added this pull request to the merge queue Dec 20, 2023
Merged via the queue into badges:master with commit 6e581b7 Dec 20, 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

3 participants