Add optional FDC3 version property to AppD record - #1965
Conversation
✅ Deploy Preview for fdc3 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
julianna-ciq
changed the title
|
picked up some conflicts and is failing out of the website build for some reason - needs investigation |
|
@kriswest I think Netlify is failing because |
|
Hmm, dependabot is supposed to have the same setting: Line 13 in 31f79d4 and it proposed that update. I guess its not applying that config to the website folder?? Would also explain why its not grouping updates for the website. I'll raise something to try and do that downgrade and fix the dependabot config. |
|
@julianna-ciq This should fix it, could you take a look: #2007 |
| }, | ||
| "fdc3Version": { | ||
| "type": "string", | ||
| "format": "semver-range", |
There was a problem hiding this comment.
Do you know what version of the json schema specification this is supported in? Can we get that added to the description.
If its not been adopted we can do this with a regex over a string type. Theres a regex for semver in here somewhere: semver/semver.org#431
There was a problem hiding this comment.
I don't see it in 2020-12: https://www.learnjsonschema.com/2020-12/format-annotation/format/
maybe a hallucination?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1965 +/- ##
==========================================
- Coverage 95.40% 95.27% -0.13%
==========================================
Files 85 85
Lines 6718 6737 +19
Branches 899 986 +87
==========================================
+ Hits 6409 6419 +10
- Misses 309 318 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Is this still a draft? can you change title if not and I'll review |
julianna-ciq
changed the title
kriswest
left a comment
kriswest
left a comment
There was a problem hiding this comment.
The regex for validation needs testing and needs a few tweaks - we should lose the v1 appD spec at the same time
| "type": "string", | ||
| "description": "Version of the application. This allows multiple app versions to be defined using the same app name. This can be a triplet but can also include things like 1.2.5 (BETA)" | ||
| }, | ||
| "fdc3Version": { |
There was a problem hiding this comment.
This shouldn't be getting added to the v1 app spec. However, we could also remove that entirely as its overdue to go along with the deprecated routes.
| const range = semver.validRange(appFdc3Version); | ||
| return range != null && semver.satisfies(fdc3Version, range); |
There was a problem hiding this comment.
This isn't using the regex defined in the App Directory schema and probably should be...
There was a problem hiding this comment.
Can we validate using teh schema OR at least the regex from the schema
3 participants
Summary
fdc3Versionmetadata as an npm-style semver rangeFixes #1871
Verification
npx.cmd tsc -p toolbox/fdc3-for-web/fdc3-web-impl/tsconfig.json --noEmitnpm.cmd test --workspace toolbox/fdc3-for-web/fdc3-web-impl -- BasicDirectoryreaches compile, then stops becausevitestis not installed in the local checkout