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

[v8] Print max-in-flight value alongside deployment status #3097

Merged
merged 5 commits into from
Aug 13, 2024

Conversation

weresch
Copy link
Contributor

@weresch weresch commented Aug 12, 2024

Description of the Change

This prints max-in-flight: <value> after the deployment status, when there is an Active deployment for an app and when max-in-flight is set to a non-default value

With #3085 merged into v8, this PR takes the new changes from #3088 and targets the v8 branch.

Why Is This PR Valuable?

It provides the user with information/confirmation that they have set a non-default max-in-flight value

+@joaopapereira @weresch @pivotalgeorge

pivotalgeorge and others added 2 commits August 12, 2024 10:22
Co-authored-by: Greg Weresch <greg.weresch@broadcom.com>
…s table

when it is set to non-default value (currently 1)

Co-authored-by: Greg Weresch <greg.weresch@broadcom.com>
a-b
a-b previously approved these changes Aug 12, 2024
Copy link
Member

@a-b a-b left a comment

Choose a reason for hiding this comment

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

Looks clean and well written.

Copy link
Contributor

@joaopapereira joaopapereira left a comment

Choose a reason for hiding this comment

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

The change in general looks good.
Added some formatting nits(I think we should address these to try to get a consistently formated codebase) and some questions.

command/v7/shared/app_summary_displayer.go Show resolved Hide resolved
command/v7/shared/app_summary_displayer_test.go Outdated Show resolved Hide resolved
command/v7/shared/app_summary_displayer_test.go Outdated Show resolved Hide resolved
command/v7/shared/app_summary_displayer_test.go Outdated Show resolved Hide resolved
command/v7/shared/app_summary_displayer_test.go Outdated Show resolved Hide resolved
- these tests depend on a short-lived state
Copy link
Contributor

@joaopapereira joaopapereira left a comment

Choose a reason for hiding this comment

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

LGTM

@a-b a-b merged commit 59cfa92 into cloudfoundry:v8 Aug 13, 2024
20 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.

4 participants