-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add [Raycast] Badge #9801
Conversation
Fatpandac
commented
Dec 11, 2023
|
services/raycast/installs.service.js
Outdated
async fetch({ user, extension }) { | ||
return this._requestJson({ | ||
schema, | ||
url: `https://www.raycast.com/frontend_api/users/${user}/extensions/${extension}`, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this 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.
🚀 Updated review app: https://pr-9801-badges-shields.fly.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
latest changes lgtm 👍