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

[Harmless] K8petstore shell update, separated from the larger PR #8678

Merged
merged 1 commit into from May 29, 2015

Conversation

jayunit100
Copy link
Member

Here is a quick update to k8petstore. Ive separated it from the other PR because people are now using it and this will be an easy review for folks (it only touches one file, in examples) :)

@k8s-bot
Copy link

k8s-bot commented May 22, 2015

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain @ixdy.

@jayunit100 jayunit100 changed the title K8petstore shell update, separated from the larger PR [Harmless] K8petstore shell update, separated from the larger PR May 22, 2015
@jayunit100
Copy link
Member Author

@chuckbutler has tested this iirc.
And, mister chucky, after we get this in, we can then lob any new ideas/changes (suggested in ) on top.
im happy help to review any patch you have.

@saad-ali
Copy link
Member

Fixing example ok for 1.0. Assigning reviewer.

@lazypower
Copy link
Contributor

@jayunit100 sounds good to me. I haven't had a chance to circle back to this yet w/ any of the modifications, but should have some time on Thursday to give it all a good look/run-through for our action(s) usecase.

Thanks!

@eparis
Copy link
Contributor

eparis commented May 27, 2015

LGTM with whitespace fix on like 23{1,2,3}

Also, cleaned up whitespace inconsistencies.
@jayunit100
Copy link
Member Author

yup, okay fixed the whitespaces needed some WS cleanups, done ! thanks @eparis

@erictune
Copy link
Member

if this LGTeparis then it LGTM.

@eparis eparis 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 May 29, 2015
@eparis
Copy link
Contributor

eparis commented May 29, 2015

what are the shippable failures?

@erictune
Copy link
Member

I chose to ignore shippable because you can't see the failures without giving them excessive github permissions.

The travis failures appear to be a failure during race checking. That can't be related to this PR.

@eparis
Copy link
Contributor

eparis commented May 29, 2015

both 1.3 and 1.4

ok      github.com/GoogleCloudPlatform/kubernetes/pkg/securitycontext   0.041s  coverage: 62.5% of statements
--- FAIL: TestNoManualChangesToGenerateDeepCopies (0.27s)
    conversion_generation_test.go:126: please update conversion functions; generated: api.deep_copy.txt; first diff: 
        expected:   if in.ISCSI != nil {

             got:   return nil
    conversion_generation_test.go:126: please update conversion functions; generated: v1beta3.deep_copy.txt; first diff: 
        expected:   if in.ISCSI != nil {

             got:   return nil
    conversion_generation_test.go:126: please update conversion functions; generated: v1.deep_copy.txt; first diff: 
        expected:   if in.ISCSI != nil {

             got:   return nil
FAIL
coverage: 81.5% of statements

@eparis eparis added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 29, 2015
rjnagal added a commit that referenced this pull request May 29, 2015
[Harmless] K8petstore shell update, separated from the larger PR
@rjnagal rjnagal merged commit 8e23407 into kubernetes:master May 29, 2015
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants