add [VpmVersion] badge #8755
add [VpmVersion] badge #8755
Conversation
services/vpm/vpm-version.service.js
Outdated
| packageId: 'com.vrchat.udonsharp', | ||
| }, | ||
| queryParams: { | ||
| repositoryUrl: 'https://packages.vrchat.com/curated?download', |
There was a problem hiding this comment.
I realise that this is a user-supplied parameter so the user can pass in whatever they want, but for a version badge it seems like you could pass in https://packages.vrchat.com/curated (without ?download) here and works just the same. For the purposes of the example/test, do we need this?
There was a problem hiding this comment.
The reason why I use url with ?download is it's the actual url vpm uses.
services/vpm/vpm-version.service.js
Outdated
| if (sort === 'defined') { | ||
| version = versions.reverse()[0] | ||
| } else if (sort === 'semver') { | ||
| version = latest(versions) | ||
| } |
There was a problem hiding this comment.
Can you expand a bit on the reason for allowing multiple sort methods here? I don't really know the norms of this community, but for package registries we generally want the concept of 'latest' to align with that of the upstream registry (What version would I get if I run vpm install, or whatever?). We do have this for some badges like github tags/releases where the platform is unaware of the language ecosystem norms or version schema (a github release could be anything - an NPM package, an android app, whatever..). For package registries (NPM, Ruby gems, Rust crates, PyPI, etc) we don't generally do this.
There was a problem hiding this comment.
Also, normally for version badges, /v would be the latest version (excluding pre-releases) and we'd have an include_prereleases flag for latest version (including pre-releases). Does VPM have any concept of stable/pre-releases?
There was a problem hiding this comment.
Because official repository template is made based on GitHub releases, in most case, its ordered by release date. however, it's not documented & not a rule for the repository so we can remove this sort.
I think this sort of useful in case of releasing multiple major releases but it may not be a enough reason.
Does VPM have any concept of stable/pre-releases?
Yes. VPM is based on semver. I forget to check prereleases in defined sort.
services/vpm/vpm-version.service.js
Outdated
| import { BaseJsonService, NotFound } from '../index.js' | ||
|
|
||
| const queryParamSchema = Joi.object({ | ||
| repositoryUrl: optionalUrl.required(), |
There was a problem hiding this comment.
Can we make the query param repository_url. To make the variable name repositoryUrl internally, you can change the function signature for handle() to
async handle({ packageId }, { repository_url: repositoryUrl, sort }) {(you'll need to update the examples to use repository_url)
services/vpm/vpm-version.service.js
Outdated
| .pattern( | ||
| /./, | ||
| Joi.object({ | ||
| versions: Joi.object().pattern(/./, Joi.object()).required(), |
There was a problem hiding this comment.
We should make this
versions: Joi.object().pattern(/./, Joi.object()).min(1).required(),This will require the versions object to have at least one key. At the moment, Object.keys(pkg.versions) could be [].
| @@ -0,0 +1,45 @@ | |||
| import { isSemver } from '../test-validators.js' | |||
| import { createServiceTester } from '../tester.js' | |||
| export const t = await createServiceTester() | |||
There was a problem hiding this comment.
The service tests didn't run in CI. This is a known issue - we're working on it, but we still have one foot in GitHub actions and one foot in Circle CI. Lets not get too bogged down in that, but I ran the service tests locally and they all failed. Can you have a look at getting the tests passing. You'll need to run them locally.
There was a problem hiding this comment.
Sorry, i did ran npm run test but not ran npn run test:services. I fixed tests.
| repository_url: 'https://packages.vrchat.com/curated?download', | ||
| }, | ||
| staticPreview: renderVersionBadge({ version: '1.1.6' }), | ||
| }, |
There was a problem hiding this comment.
Can we add a second example with the include_prereleases flag?
| repository_url: 'https://packages.vrchat.com/curated?download', | ||
| include_prereleases: null, | ||
| }, | ||
| staticPreview: renderVersionBadge({ version: '1.1.6' }), |
There was a problem hiding this comment.
should I use fake prerelease version like 1.1.7-beta.1 for static preview?
There was a problem hiding this comment.
I think it is fine as it stands
|
Closes #8752