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

Use bash ranges "{1..3}" instead of "$(seq 1 3)". #26790

Merged
merged 1 commit into from
Aug 9, 2016
Merged

Use bash ranges "{1..3}" instead of "$(seq 1 3)". #26790

merged 1 commit into from
Aug 9, 2016

Conversation

rmmh
Copy link
Contributor

@rmmh rmmh commented Jun 3, 2016 •
edited by k8s-oncall
Loading

Analytics

This change is Reviewable

@rmmh rmmh added 0 - Backlog release-note-none Denotes a PR that doesn't merit a release note. labels Jun 3, 2016
@rmmh rmmh assigned ixdy Jun 3, 2016
@ixdy ixdy added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 3, 2016
@ixdy
Copy link
Member

ixdy commented Jun 3, 2016

on the list of code cleanups this is pretty close to the bottom, but ok. :)

@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 3, 2016
@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 18, 2016
@ixdy
Copy link
Member

ixdy commented Jun 29, 2016

@rmmh ping

@rmmh rmmh force-pushed the bash-range branch 2 times, most recently from f67181e to 2e0b6e4 Compare June 29, 2016 21:22
@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 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 Jun 29, 2016
@rmmh
Copy link
Contributor Author

rmmh commented Jun 29, 2016

@ixdy SYN-ACK

On Wed, Jun 29, 2016 at 2:39 PM, k8s-merge-robot notifications@github.com
wrote:

PR changed after LGTM, removing LGTM.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#26790 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAM7nDWD003UstofXqF_xyTAnmMtAhdbks5qQuYRgaJpZM4ItwPz
.

@ixdy
Copy link
Member

ixdy commented Jun 29, 2016

Looks like a few new places slipped in:

  • hack/lib/util.sh
  • hack/jenkins/e2e-runner.sh
  • a number of spots in test/

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 13, 2016
@ixdy ixdy assigned rmmh and unassigned ixdy Aug 8, 2016
@ixdy
Copy link
Member

ixdy commented Aug 8, 2016

@rmmh please assign back after rebase and comments addressed

@rmmh rmmh assigned ixdy and unassigned rmmh Aug 8, 2016
@rmmh
Copy link
Contributor Author

rmmh commented Aug 8, 2016 •
edited
Loading

Rebased and addressed.

Note that bash brace expansion happens before variable expansion, so $(seq 1 $times) isn't the same as {1..$times}

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2016
@k8s-bot
Copy link

k8s-bot commented Aug 8, 2016

GCE e2e build/test passed for commit 5508e49.

@ixdy
Copy link
Member

ixdy commented Aug 9, 2016

LGTM

@ixdy ixdy added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 9, 2016
@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 9, 2016

GCE e2e build/test passed for commit 5508e49.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit cdee52e into kubernetes:master Aug 9, 2016
@k8s-github-robot k8s-github-robot mentioned this pull request Aug 9, 2016
Closed
@rmmh rmmh deleted the bash-range branch August 29, 2016 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ixdy ixdy

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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rmmh @ixdy @k8s-bot @k8s-github-robot @googlebot