-
-
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
Finish moving hostnames from routes to query strings #3714
Comments
Another simpler example of such migration is this commit I did in #3633 (though the redirector bit will be missing) |
Sorry - diving in late to the party on this one. In general, I think there are 2 different cases to consider when we're thinking about this issue. For something like BitBucket or NPM there is a canonical instance ( https://bitbucket.org , https://www.npmjs.com/ etc), but you can also host your own. In that case, it makes sense to default to the canonical instance but allow a self-hosted instance to be passed in the query string. e.g: https://img.shields.io/npm/v/npm.svg The other case is something which is inherently self-hosted or where there isn't a centralised or default version. Something like Jenkins or JIRA fits into this box. In that case, you always need to specify the instance, so we've tended to use the e.g: https://img.shields.io/jira/issue/https/issues.apache.org/jira/KAFKA-2896.svg Are you looking to move to always putting the host/instance in the query string for (even when its always required) now? |
Probably the majority are using
Yes, that's what I'm proposing. So (extensionless) https://img.shields.io/jira/issue/https/issues.apache.org/jira/KAFKA-2896 would become https://img.shields.io/jira/issue/KAFKA-2896?server=https://issues.apache.org/jira/ (which IMO is easier to make sense of visually) An exception could be cases like the hsts preload badge, where there's no protocol, and it's a domain (i.e. a fixed number of path components), not a URL: https://img.shields.io/hsts/preload/github.com I've a moderate preference to stick complicated things like URLs into the query string rather than defining them using a variable number of path components. I think the variable-length path components make our URL patterns more complicated to implement and harder to read, and require learning our convention (though it's admittedly not a terribly complicated one). I'm certainly open to discussing it! Added: Another early example: #1277 (comment) |
Its slightly odd that the base route does nothing on its own and server becomes a required param. I think these badges would be the only example of that we have? ..but then the I suppose it does at least always give us a bit of future-proofing if Atlassian launch JIRA Cloud, or something like that. |
Already exists 😉 https://www.atlassian.com/software/jira/pricing |
Well, it'll be easier to accommodate it soon then :D |
Nexus was handled in #3792 |
Closing this out as all the PRs for the listed services have been merged |
Didn't have much involvement in this piece of work apart from Bugzilla I did a while back, but great job, the migrations were impressively fast! 👍 |
Any reason why we didn't migrate Jenkins in the end (given that we migrated other "decentralised" services such as Jira)? If it's an oversight, I'm happy to pick it up. |
Just an oversight, #4178 was opened to include some of the ones that were missed |
I had missed that issue, will comment on it. |
As a number of services were migrated to the modern service architecture, the component of the route that held a URL – the hostname, and sometimes path and protocol – was moved into a single
?server=
query parameter.This is the argument I made when I updated Sonar in #3696 :
There are a few more of these which need updating:
One of these, Teamcity, is still using the deprecated
format
argument with a regex string. It should be converted to use apattern
, probably at the same time.Anyone is welcome to pick one of these up! You can look at the Sonar PR as a (relatively complicated) example of what this looks like. The services should be updated to reflect the new route and query params, and a redirector added to support backward compatibility with the old route.
If you want to tackle one of these, post a note here first so we don't duplicate effort! Also feel free to post if you have any questions!
The text was updated successfully, but these errors were encountered: