Skip to content
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

Decide on what we should call "base url" parameters #10806

Open
chris48s opened this issue Jan 10, 2025 · 4 comments
Open

Decide on what we should call "base url" parameters #10806

chris48s opened this issue Jan 10, 2025 · 4 comments
Labels
developer-experience Dev tooling, test framework, and CI

Comments

@chris48s
Copy link
Member

chris48s commented Jan 10, 2025

Over the last 10+ years we have accumulated a lot of code written by a lot of different people.
In general, I think we've done a reasonable job of enforcing relatively consistent code styles and naming conventions, via documentation and/or lint rules, but there are obviously some exceptions.

One significant "blind spot" where we have a lot of variance is what we call "base url" type parameters.

Related to #10805

We've got a bunch of services that call this param server

queryParam({
name: 'server',
example: 'https://drone.shields.io',
}),

Some that call it instance

queryParam({ name: 'instance', example: 'https://api.opensuse.org' }),

baseUrl or base_url is quite common

queryParam({
name: 'baseUrl',
example: 'https://bugs.eclipse.org/bugs',
description:
'When not specified, this will default to `https://bugzilla.mozilla.org`.',
}),

..and then some services include the name of the service e.g:

queryParam({
name: 'revolt_api_url',
example: 'https://api.revolt.chat',
}),

queryParam({
name: 'gitlab_url',
example: 'https://gitlab.com',
}),

Fundamentally, all of these describe the exact same concept. I think there is value in making this horizontally consistently across badges and I think the reasons why we have a lot of variance here is not down to trying to maintain consistency with upstream terminology.

There are a couple of ways we can change the names of query params without making a breaking change. One way we can do it is with redirects. Another would be to write the queryParamSchemas to accept both formats but only document one for the services where we want to fix this. We don't have loads of these, but I probably wouldn't want to do all of them in one PR. It would be nice to gradually work towards fixing this though.

I suggest we:

  1. Decide what we are going to call this concept
  2. Document it
  3. Maybe make some kind of a lint/danger rule to try and catch this, if possible?
  4. At least get to a stage where everything we add from now onwards matches this convention
  5. Bring existing services into line with the convention

IMO, we should pick a generic term: "server", "instance" or "baseUrl" (i.e: we should not have the service name in the param name). Beyond that, I don't really have a super strong opinion on which name we pick.

@chris48s
Copy link
Member Author

Just thinking about this a bit more (particularly in the context of #10792 and #10805 ) I think baseUrl makes more sense as a name for this concept than server or instance.

In the case where the URL includes an endpoint (i.e: your thing lives at https://bugs.eclipse.org/bugs or https://issues.apache.org/jira or whatever) baseUrl feels more right than server or instance.

@jNullj
Copy link
Member

jNullj commented Jan 10, 2025

Should we force https only? I like more the idea offered at #2637 where we would still support http but make it harder to use, maybe add a warning in docs near this option.

Regarding the name, i am for something like endpointUrl but i don't object baseUrl i think it still makes sense.

@cyb3rko
Copy link
Contributor

cyb3rko commented Jan 10, 2025

I would go with baseUrl but I'm also fine with endpointUrl.

But as already mentioned in #10792, Url would only fit if we include the scheme inside the parameter (referring to #10805).

@chris48s
Copy link
Member Author

would only fit if we include the scheme inside the parameter

Yeah 👍 so to summarise #10805

  • This param should always be a full URL including protocol://
  • We should bring the two exceptions to this (matrix, mastodon) into line
  • If we want to enforce HTTPS, we can do it with validation in the queryParam schema Joi.string().uri({ scheme: ['https'] })

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Dev tooling, test framework, and CI
Projects
None yet
Development

No branches or pull requests

3 participants