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

Deployment: Updating API comments #15273

Merged
merged 1 commit into from
Oct 8, 2015

Conversation

nikhiljindal
Copy link
Contributor

Updating some API comments:

  • Updating the comment for Deployment.Spec.Selector to clarify that the new pods are also selected by it.
  • Updating comments for Status.Replicas to clarify that it only reflects the number of pods that have been created (it does not check for readiness), based on discussion: Adding scale up down code for DeploymentController #14209 (comment)

cc @bgrant0607 @ironcladlou @ghodss

@k8s-github-robot k8s-github-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 7, 2015
@k8s-github-robot
Copy link

Labelling this PR as size/S

@k8s-bot
Copy link

k8s-bot commented Oct 8, 2015

GCE e2e build/test failed for commit d5dd4b9b7d788415a7f9062d2bcd9403ca1b76d9.

@@ -279,11 +279,11 @@ type RollingUpdateDeployment struct {
}

type DeploymentStatus struct {
// Total number of ready pods targeted by this deployment (this
// Total number of pods targeted by this deployment (this
Copy link
Member

Choose a reason for hiding this comment

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

Technically, it doesn't include terminated pods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the comment to say "number of non-terminated pods"

@nikhiljindal
Copy link
Contributor Author

@bgrant0607 Updated the code as per comment. PTAL

@bgrant0607
Copy link
Member

LGTM

@bgrant0607 bgrant0607 added lgtm "Looks good to me", indicates that a PR is ready to be merged. e2e-not-required labels Oct 8, 2015
@k8s-bot
Copy link

k8s-bot commented Oct 8, 2015

GCE e2e test build/test passed for commit b3e659a260436a2e41e8179895e80f5c4701f6cb.

@k8s-bot
Copy link

k8s-bot commented Oct 8, 2015

GCE e2e test build/test passed for commit 3e543d7ec2c0c8ea9e1bc1f393c1e90f23cc9717.

@nikhiljindal
Copy link
Contributor Author

Ran hack/update-generated-swagger-docs.sh to update generated swagger doc.
Re-adding LGTM.

@nikhiljindal nikhiljindal added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 8, 2015
@k8s-bot
Copy link

k8s-bot commented Oct 8, 2015

GCE e2e test build/test passed for commit 119150f.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Oct 8, 2015
@k8s-github-robot k8s-github-robot merged commit 79b70df into kubernetes:master Oct 8, 2015
k8s-github-robot referenced this pull request Oct 13, 2015
…#15273-upstream-release-1.1

Auto commit by PR queue bot
shyamjvs referenced this pull request in shyamjvs/kubernetes Dec 1, 2016
…y-pick-of-#15273-upstream-release-1.1

Auto commit by PR queue bot
shouhong referenced this pull request in shouhong/kubernetes Feb 14, 2017
…y-pick-of-#15273-upstream-release-1.1

Auto commit by PR queue bot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants