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.

Sign up for GitHub

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

Jump to bottom

Add MinReadySeconds in deployment configs #9852

Merged
merged 3 commits into from
Jul 18, 2016
Merged

Add MinReadySeconds in deployment configs #9852

merged 3 commits into from
Jul 18, 2016

Conversation

0xmichalis
Copy link
Contributor

@0xmichalis 0xmichalis commented Jul 14, 2016 •
edited
Loading

@0xmichalis
Copy link
Contributor Author

[test]

@0xmichalis
Copy link
Contributor Author

#9888 [test]

@0xmichalis
Copy link
Contributor Author

[test]

mfojtik
mfojtik reviewed Jul 18, 2016
View reviewed changes
pkg/deploy/api/types.go
@@ -297,6 +297,11 @@ type DeploymentConfigSpec struct {
// Strategy describes how a deployment is executed.
Strategy DeploymentStrategy

// Minimum number of seconds for which a newly created pod should be ready
Copy link
Contributor

Choose a reason for hiding this comment

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

Make this comment as MinReadSeconds is the minimum...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@mfojtik
Copy link
Contributor

mfojtik commented Jul 18, 2016

@Kargakis minor suggestions, but the code looks great! Can you also please add minReadySeconds into oc describe dc ?

@0xmichalis
Copy link
Contributor Author

Comments addressed - @openshift/api-review can I get an approval for this? Last upstream feature and then DeploymentConfigs are a superset of Deployments.

@smarterclayton
Copy link
Contributor

API approved

@mfojtik
Copy link
Contributor

mfojtik commented Jul 18, 2016

LGTM

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 1b0111d

@mfojtik mfojtik added the lgtm Indicates that a PR is ready to be merged. label Jul 18, 2016
@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6382/)

@0xmichalis
Copy link
Contributor Author

[merge]

1 similar comment
@mfojtik
Copy link
Contributor

mfojtik commented Jul 18, 2016

[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 18, 2016 •
edited
Loading

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6409/) (Image: devenv-rhel7_4616)

@smarterclayton
Copy link
Contributor

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 1b0111d

@openshift-bot openshift-bot merged commit 312dc77 into openshift:master Jul 18, 2016
@0xmichalis 0xmichalis deleted the mindreadyseconds branch July 19, 2016 07:53
@smarterclayton smarterclayton mentioned this pull request Aug 5, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mfojtik mfojtik

Labels
api-approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@0xmichalis @mfojtik @smarterclayton @openshift-bot