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

Refactoring runner resource container linedelimiter to it's own pkg #26958

Merged
merged 4 commits into from
Aug 3, 2016
Merged

Refactoring runner resource container linedelimiter to it's own pkg #26958

merged 4 commits into from
Aug 3, 2016

Conversation

resouer
Copy link
Contributor

@resouer resouer commented Jun 7, 2016

Continuing my work ref #15634

Anyone is ok to review this fix.

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Jun 7, 2016
@k8s-github-robot
Copy link

@resouer
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

@resouer
Copy link
Contributor Author

resouer commented Jun 8, 2016

ping @thockin , submit queue is back, please review.

@thockin thockin added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Jun 9, 2016
@thockin
Copy link
Member

thockin commented Jun 9, 2016

LGTM. Not critical for 1.3, though.

@resouer
Copy link
Contributor Author

resouer commented Jun 13, 2016

@k8s-bot unit test this issue: #27179

@resouer
Copy link
Contributor Author

resouer commented Jun 16, 2016

@thockin What is merge rule for Milestone is for a future release and cannot be merged, after 1.3?

@fejta
Copy link
Contributor

fejta commented Jun 27, 2016

@k8s-bot node e2e test this issue: #28047

thockin
thockin reviewed Jun 27, 2016
View reviewed changes
pkg/util/linedelimiter/line_delimiter.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package util
package linedelimiter
Copy link
Member

Choose a reason for hiding this comment

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

This could go in util/strings couldn't it?

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2016
@resouer
Copy link
Contributor Author

resouer commented Jul 3, 2016

@thockin Fixed and rebased, please review

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 3, 2016
thockin
thockin reviewed Jul 3, 2016
View reviewed changes
pkg/util/async/runner.go
@@ -1,5 +1,5 @@
/*
Copyright 2014 The Kubernetes Authors.
Copyright 2014 The Kubernetes Authors All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to remove the "All rights reserved" from these - new boilerplate has removed it.

@thockin
Copy link
Member

thockin commented Jul 3, 2016

LGTM except jenkins (probably the "all rights reserved". Thanks! Sometimes less new packages is better :)

@thockin
Copy link
Member

thockin commented Jul 3, 2016

ping me when CI is green, please

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 4, 2016
@resouer
Copy link
Contributor Author

resouer commented Jul 26, 2016

ping @thockin, this is ready to be merged

@thockin thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 26, 2016
@k8s-github-robot k8s-github-robot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. retest-not-required-docs-only and removed retest-not-required-docs-only labels Jul 28, 2016
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 29, 2016
@resouer
Copy link
Contributor Author

resouer commented Aug 2, 2016 •
edited
Loading

kindly ping @thockin, please set label back :)

@thockin thockin removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Aug 3, 2016
@thockin
Copy link
Member

thockin commented Aug 3, 2016

LGTM

@thockin thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 3, 2016
@k8s-bot
Copy link

k8s-bot commented Aug 3, 2016

GCE e2e build/test passed for commit e0ab760.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Aug 3, 2016

GCE e2e build/test passed for commit e0ab760.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 24216fe into kubernetes:master Aug 3, 2016
@resouer resouer deleted the util-line branch August 3, 2016 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@thockin thockin

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@resouer @k8s-github-robot @thockin @fejta @k8s-bot @googlebot