-
Notifications
You must be signed in to change notification settings - Fork 91
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
chore: use 'https' protocol for .net 6.0 devfile. Update schema to 2.2.0 #298
Conversation
@kadel @yangcao77 folks, could you clarify why 2.2.2 is not supported ?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kadel as you are the owner of the stack, could you please also review the PR?
My bad I've tested with schemaVersion 2.2.0 instead of 2.2.2. I've removed my approval 🥲 |
@thepetk sure I can changed the version, but for me it looks like the problem should be fixed on the parser end. |
/retest |
@ibuziuk FYI I've created devfile/api#1454 in order to update the parser and add support for devfile schema - versions |
1f99545
to
58a86b4
Compare
/retest |
1 similar comment
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ibuziuk I've tested locally the new devfile and it looks good to me.
One minor detail I would suggest is to create a new default version (e.g. 1.0.3
) of the dotnet60
instead of directly updating the 1.0.2
. The updated stacks/dotnet60
should have the following contents:
- stack.yaml
- 1.0.2/
- devfile.yaml
...
- 1.0.3/
- devfile.yaml
...
@thepetk could you clarify why would you like to create intricate versioning in the repo and not just bump the version to 1.0.3 ? e.g. I do not see 1.0.1 / 1.0.0 versions atm |
@ibuziuk IMO this would be a better approach and is followed for other stacks (e.g |
@thepetk sounds good, thank you |
…e latest 2.2.0 Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
9611bc9
to
f00df79
Compare
@thepetk can the PR be merged now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ibuziuk, kadel, thepetk The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What does this PR do?:
Which issue(s) this PR fixes:
N/A
PR acceptance criteria:
Have you read the devfile registry contributing guide and followed its instructions?
Does this repository's tests pass with your changes?
Does any documentation need to be updated with your changes?
Have you tested the changes with existing tools, i.e. Odo, Che, Console? (See devfile registry contributing guide on how to test changes)
How to test changes / Special notes to the reviewer:
try it out on Developer Sandbox - https://workspaces.openshift.com/#https://raw.githubusercontent.com/devfile/registry/1f99545009bad5ab1f3231c66bcd6bf29d583573/stacks/dotnet60/devfile.yaml