-
Notifications
You must be signed in to change notification settings - Fork 65
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
[wip] add RunOnDemand changes in devfile api. It is not ready to be merged. #880
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rajibmitra The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: rajibmitra <[email protected]>
Signed-off-by: rajibmitra <[email protected]>
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.
As discussed in the community call on June 20th, this is a breaking change and inverts the current default for container components. I don't think it's suitable to merge (with a default value of false
) without moving to a new API version.
Thanks @amisevsk, any idea when we are planning to move to new API version ? |
The new API version in question would likely need to be v3.0.0, which is a very significant change that as far as I know isn't on the horizon yet. If the default here was |
@l0rd can we make default here |
@rajibmitra
Regarding on when to bump up the version, #46 is the only issue we left for devfile 2.2. we should think of a backward compatible design for the next |
thank you @yangcao77 for the clarification. |
@amisevsk I must admit that the name is confusing but But, as mentioned by @yangcao77, on the 20th of June Devfile Cabal, we have discussed some different syntax and @kadel is looking at the proposal but is currently on vacation so we need to be patient, and I don't think that there is any urgency. @rajibmitra until we do not have an agreement on the syntax I would recommend you to:
|
Just making sure that everyone is aware of this: The updated proposal is at #852 (comment) |
Closing this PR as its been WIP for 2 years. |
What does this PR do?:
This is the changes for RunOnDemand in container component.
Which issue(s) this PR fixes:
#852