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

S2i 5009651 bump #11414

Merged
merged 2 commits into from
Oct 20, 2016
Merged

S2i 5009651 bump #11414

merged 2 commits into from
Oct 20, 2016

Conversation

jim-minter
Copy link
Contributor

No description provided.

@jim-minter jim-minter assigned jim-minter and bparees and unassigned jim-minter Oct 18, 2016
@bparees
Copy link
Contributor

bparees commented Oct 18, 2016

[test]

@bparees
Copy link
Contributor

bparees commented Oct 18, 2016

[testextended][extended:core(builds)]

@bparees
Copy link
Contributor

bparees commented Oct 18, 2016

[testextended][extended:core(image_ecosystem)]

@bparees
Copy link
Contributor

bparees commented Oct 18, 2016

@PI-Victor fyi

@openshift-bot
Copy link
Contributor

Evaluated for origin testextended up to 6c4d26e

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 6c4d26e

// return &TimeoutError{}
// }
// timer.Reset(timeout)
func TimeoutAfter(t time.Duration, errorMsg string, f func(*time.Timer) error) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

@bparees @jim-minter thanks for the merge.
just a curiosity, this whole thing, couldn't we replace it with this: https://golang.org/pkg/time/#AfterFunc
or am i reading the scope of the function wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PI-Victor
time.AfterFunc waits T, then runs f()
this function runs f(), concurrently waits T and returns if f() hasn't finished by then.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jim-minter yeah, you're right. makes sense now.
thanks for the clarification

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10183/) (Base Commit: f6bd74b)

@PI-Victor
Copy link
Contributor

@jim-minter thanks for patching the holes in openshift in this PR introduced by me, i completely forgot to mention that :)

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/testextended FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin_extended/624/) (Base Commit: 5134734) (Extended Tests: core(image_ecosystem), core(builds))

@jim-minter
Copy link
Contributor Author

@bparees
Copy link
Contributor

bparees commented Oct 20, 2016

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 20, 2016

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

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 6c4d26e

@openshift-bot openshift-bot merged commit c49fe4c into openshift:master Oct 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants