badges / shields

Concise, consistent, and legible badges in SVG and raster format
https://shields.io
Creative Commons Zero v1.0 Universal
23.75k stars 5.5k forks source link

Deprecate route `format`, replacing with `pattern` #3329

Open paulmelnikow opened 5 years ago

paulmelnikow commented 5 years ago

To make the code easier to understand and to support trie-based routing for #3328, it would be helpful to replace the several remaining instances of route formats with patterns.

Many of the remaining formats are designed to support aliases, and since we've started using redirects heavily, the likely solution is to rewrite the canonical form using a pattern, and add redirects for each of the alias forms.

Ideally by using multiple redirects we can rewrite everything using patterns.

paulmelnikow commented 5 years ago

With the PRs for #3714 merged, outside of legacy redirectors there aren't many of these left!

calebcartwright commented 5 years ago

One of the last remaining non-redirect ones is Wercker. I've sat down twice now to attempt to convert but it's a bit of an 👀 popper.

https://github.com/badges/shields/blob/ddbe19f6bd2f1b81c37712764a93edd9ac9c4b83/services/wercker/wercker.service.js#L22-L29

Based on the dialog in https://github.com/badges/shields/issues/2076, my sense is that there's two different types of build status we support from Wercker that require two different types of input (one an ID and the other the user/app name combo).

I'm thinking it might be easiest to split these in two, at least the service route definitions, with a base class/reusable functions for all the common bits.

Does that sound reasonable?

chris48s commented 5 years ago

There's an explanation of why splitting it is hard at https://github.com/badges/shields/pull/2279#discussion_r231661787 Once you split them into 2 services, we can't control the order the routes get matched in.

That said, as a related issue, what we're doing on the wercker badge is also a bit strange anyway (see point 3 on https://github.com/badges/shields/pull/2103 ). One of the routes that exists does an unexpected thing for legacy compatibility reasons.

At the time we did that, we had no idea how many users might be relying on the wercker badges, but now we have stats we know that we've served less than 1,000 total requests for the wercker badges in the last month.

Given all that, I'd be happy to:

paulmelnikow commented 4 years ago

Another of these where we're still using a format is Travis. We're using a negative lookahead to prevent matching php-v, along with an optional domain component. I'd suggest we resolve this one by making the domain org|com required, and adding a redirector with the legacy format.