-
Notifications
You must be signed in to change notification settings - Fork 992
[v8] RFC-0045: Stack management #3687
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
base: v8
Are you sure you want to change the base?
Conversation
Signed-off-by: Simon Jones <simonjones@vmware.com>
Signed-off-by: Simon Jones <simonjones@vmware.com>
Signed-off-by: Simon Jones <simonjones@vmware.com>
Signed-off-by: Simon Jones <simonjones@vmware.com>
Signed-off-by: Simon Jones <simonjones@vmware.com>
Signed-off-by: Simon Jones <simonjones@vmware.com>
Signed-off-by: Simon Jones <simonjones@vmware.com>
Signed-off-by: Simon Jones <simonjones@vmware.com>
vuil
left a comment
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.
Thanks for the changes. I added a couple of minor suggestions. I assume this needs to wait till the cloud-controller changes are merged?
rashedkvm
left a comment
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.
Just a reminder to update the minimum API version prior to merging. Much appreciated!
vuil
left a comment
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.
There were some recent CI fixes that should move the integ tests along now.
Changes looks good to me. My comments are mostly minor indentation issues (probably also present in #3688 as well)
| fakeActor.GetStackByNameReturns(stack, v7action.Warnings{}, nil) | ||
| }) | ||
|
|
||
| It("Displays the stack information without state", func() { |
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.
Thanks for addressing the comments here.
nit: it's odd I am not seeing the issue on the github page, but when I checked out the branch, it is showing this It("Displays... block is misindented by one fewer level. Same is true the next It(Displays.. block
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.
I'm currently seeing correct indentation both locally and on the GH page
Co-authored-by: Cursor <cursoragent@cursor.com>
Thank you for contributing to the CF CLI! Please read the following:
If your contribution falls under a company CLA but your membership is not public, expect delays while we confirm.
Note: Please create separate PR for every branch (main and v8) as needed.
Description of the Change
This PR covers the CLI implementation details for RFC-0045: Enhance Stack Handling in Cloud Foundry. It adds an additional state field to output of the
cf stack,cf stacks&cf update-stackcommands, as well as adding the ability to modify the state to thecf update-stackcommand. This is a partner PR to the existing Cloud Controller changes submitted for reviewWhy Is This PR Valuable?
These changes will be required for consumers to interact with the new features available in the Cloud Controller, the RFC has been accepted, indicating the changes are valuable.
Applicable Issues
N/A
How Urgent Is The Change?
This should be accepted and released in line with the changes being proposed to the Cloud Controller
Other Relevant Parties
The Cloud Controller changes linked above should detail other people impacted.