-
Notifications
You must be signed in to change notification settings - Fork 931
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
cf revision command implementation [main] #3341
base: main
Are you sure you want to change the base?
Conversation
|
180f2c9
to
4057246
Compare
Co-authored-by: Lisa Burns <lisaburns@vmware.com> Signed-off-by: Steve Taylor <staylor@pivotal.io> Update help info related to revision command Return error when no app/revision is present
4057246
to
21c177c
Compare
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.
The PR looks good in general, added a nit and a couple of questions that we should get answered before merging
|
||
} | ||
|
||
func revisionDeployed(revision resources.Revision, deployedRevisions []resources.Revision) bool { |
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.
Any reason for this function to be the only one that is not associated with the struct?
if len(labels) > 0 { | ||
cmd.displayMetaData(labels) | ||
cmd.UI.DisplayNewline() | ||
} |
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.
Nit: Given that we are repeating this in different places I think that maybe the displayMetaData
function should be the one that has this if statement and the Execute
function would always call the function.
app.GUID, | ||
cmd.Version.Value, | ||
) | ||
cmd.UI.DisplayWarnings(warnings) |
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.
Question: If we display the warnings in this line, are we not going to repeat them in lines 69 and 81 as well?
This PR implements the revision command which the revision details of an app when given a revision version. The revision command is no longer experimental.