Skip to content

refactor: replace route's format with pattern in [StaticBadge] & modified [website]#11865

Open
jNullj wants to merge 7 commits into
badges:masterfrom
jNullj:refactor-format-into-pattern/static-badge
Open

refactor: replace route's format with pattern in [StaticBadge] & modified [website]#11865
jNullj wants to merge 7 commits into
badges:masterfrom
jNullj:refactor-format-into-pattern/static-badge

Conversation

@jNullj
Copy link
Copy Markdown
Member

@jNullj jNullj commented May 15, 2026

static-badge is the LAST to have format transformed to pattern!
after this is done and merged i will create one last PR to remove all code, tests and docs related to the old deprecated format then we could complete #3329.

i modified the code in website as well to reuse the same code in an helper module and added tests.
we should consider performance here as well

i suspect that splitDashSeparatedOptionalParams could in theory be faster then regex, but i should test this to be sure, maybe regex libs has some optimization im not using, maybe we could optimize splitDashSeparatedOptionalParams somehow.

other changes:

  • We now present a an invalid parameter error when user uses incorrect format instead of 404.
  • Brake support for the missing message format, label--blue which is still supported with the same render using label- -blue (not in wide use)

@jNullj jNullj added service-badge New or updated service badge core Server, BaseService, GitHub auth, Shared helpers labels May 15, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 15, 2026

Warnings
⚠️ This PR modified service code for website but not its test code.
That's okay so long as it's refactoring existing code.
Messages
📖 ✨ Thanks for your contribution to Shields, @jNullj!

Generated by 🚫 dangerJS against fa11e0e

jNullj added 3 commits May 21, 2026 14:59
label--blue and label- -blue produce the exact same results.
our docs indicate -- outputs - so the label--blue syntax is not consistenet with our docs for use in missing message, and is more confusing considering our logic here.
@jNullj
Copy link
Copy Markdown
Member Author

jNullj commented May 21, 2026

A few things to consider in the review

  1. We now present a an invalid parameter error when user uses incorrect format instead of 404.
  2. I brake here support for the missing message format, label--blue which is still supported with the same render using label- -blue

why brake support for missing message?
our docs never supported it, we clearly state that -- is output as -, this syntax is not very consistent and i find it confusing.
using a space to separate the too gives the same badge but has clear intent for an empty message and is consistent with our logic of - - =/= -- so it should not escape into -.

i hate it being a braking change, but i think its not in wide use, look at this search over github only 262 results show up and this include all -- usage, most of them are for 3 parts usage and are not relevant to this change

@jNullj jNullj changed the title refactor: replace route's format with pattern in [static-badge] & modified [website] refactor: replace route's format with pattern in [StaticBadge] & modified [website] May 21, 2026
@jNullj jNullj marked this pull request as ready for review May 21, 2026 15:47
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is undocumented, so i think its better as a redirect with its own path rather then adding logic to usage of badgeContent of the service


t.create('Missing message')
.get('/badge/label--blue.json')
.get('/badge/label-%20-blue.json')
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is hardly in use, users get the exact same result using a space - - instead of --.
This was not consistent with our docs and logic of escaping -- => -. So i find the that confusing and think we should drop support. for label--color syntax in favor of using label- -color

)
}

function splitDashSeparatedOptionalParams(s) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is now a helper function shared with Website

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

Labels

core Server, BaseService, GitHub auth, Shared helpers service-badge New or updated service badge

Development

Successfully merging this pull request may close these issues.

1 participant