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

schema: add compatibleNcs field to schema and filter based on ncs #37

Merged
merged 4 commits into from
Apr 3, 2024

Conversation

FilipZajdel
Copy link
Contributor

VSC-2486

Signed-off-by: Filip Zajdel <[email protected]>
Copy link

@datenreisender datenreisender left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the README.md the new property compatibleNcs should also be mentioned in this list: https://github.com/nrfconnect/ncs-app-index/blob/main/README.md?plain=1#L44-L51

site/src/sampleData.ts Outdated Show resolved Hide resolved
site/src/app/Header.tsx Show resolved Hide resolved
site/src/app/Header.tsx Outdated Show resolved Hide resolved
site/src/app/Header.tsx Outdated Show resolved Hide resolved
site/src/app/Header.tsx Outdated Show resolved Hide resolved
site/src/app/filters.ts Outdated Show resolved Hide resolved
@@ -0,0 +1,317 @@
{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this file added by mistake? Should it be in .gitignore?

Signed-off-by: Filip Zajdel <[email protected]>
@bencefr
Copy link

bencefr commented Apr 3, 2024

I was hoping that compatibleNcs could be a single semver condition instead an array of distinct versions. Semver inherently deals with the leading v that is quite madly prepended to tags, and a single string like 1.x || >=2.5.0 || 5.0.0 - 7.2.3 can qualify a given version quite well. I don't know if including the branches (i.e. main) is necessary though, so I'm a bit puzzled to fight for this.

@datenreisender
Copy link

I also thought that adding support for specifying semver ranges would be nice, because with it there would be less need to update the <app>.json files. But maybe this could also be a future addition.

And, yes, this gets tricky because tags and branches could really be named anything.

An idea:

  • We could stick with an array, || the elements together, and do a loose semver range match against the tags/branch.
  • Additionally we can make a equality check with every array element against the tags and if they match then we also include those.

With this one could e.g. write ['main', '1.x', '>=2.5.0 <3.0.0', '~4.2', '5.0.0 - 7.2.3'].

@FilipZajdel FilipZajdel added this pull request to the merge queue Apr 3, 2024
Merged via the queue into main with commit 56c2048 Apr 3, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants