Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[openuserjs] Add OpenUserJS service badges #8081
[openuserjs] Add OpenUserJS service badges #8081
Changes from 20 commits
ff1ef18
b363010
249d295
e671bc6
fc0fc6b
b3b72dd
053103c
b1503d5
5d24cba
731e412
020fe21
c07f320
5ec468b
f360788
7cb7aa2
4c508f9
e26ad9a
8e43b44
4a8a1f2
2947508
4e3440f
64cb1e8
92c79e5
31d61c2
584f4fc
a341d5a
6313d49
c8055a5
9336020
8f045ba
c55845a
422ed96
00d402c
91e1ea0
f9c6ba6
bcb042c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Which noun do you think will feel more natural to OpenUserJS users,
installs
ordownloads
? I gather based on the label value and what was done/discussed for GreasyFork that it'sinstalls
, but that has me rethinking the route a bit because for other badges that opt forinstalls
overdownloads
that's typically reflected in the route, e.g. Jenkins Plugin Installs - https://img.shields.io/jenkins/plugin/i/view-job-filtersOur docs on badge url structure do mention using the
d*
pattern for downloads or installs, but given the presence ofi
elsewhere I'll double check with the other maintainers to confirmAnd somewhat of an aside, but if we do decide to shift to
i*
for installs then we can consider updating the corresponding GreasyFork route too (we've got mechanisms to update routes while maintaining backwards compatibility, will share if and when we need to cross that bridge)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.
installs
is the terminology used by OpenUserJS. This includes usage on the website, the meta JSON, and confirmed by the OUJS admin Martii in comments elsewhere.I suppose I'll leave it as
d*
for now until that's decided?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.
Yup, leave the route as-is for now and I'll follow up with a resolution one way or another. I believe using the download nomenclature in the route is intentional but it's been a while since we last discussed and my memory is a bit fuzzy on where we landed
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.
We'll likely continue discussing the specifics of this offline, but for now I think we've reached a sufficient consensus that it's fine to go with this route as is. We can always add redirectors (our mechanism for updating routes without impacting existing users) for these badges down the road if we end up deciding to pivot on the route conventions.
I'll try to make time over the weekend to do a final pass over the subsequent updates
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.
Actually, I see after reading through the previously linked issues that the maintainers of the upstream service still have some concerns, so we'll put this on hold for now until we figure out whether there's a viable path forward or whether we should just abandon this.