Skip to content

Conversation

@damemi
Copy link

@damemi damemi commented Mar 11, 2021

This partially reverts some of the changes from #558 to provide the new functions for areas that need them, while keeping the old functions so as to not affect other repos that depend on them (like library-go, see openshift/library-go#1008)

@openshift-ci-robot openshift-ci-robot added the backports/unvalidated-commits Indicates that not all commits come to merged upstream PRs. label Mar 11, 2021
@openshift-ci-robot
Copy link

@damemi: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

@openshift-ci-robot openshift-ci-robot added the vendor-update Touching vendor dir or related files label Mar 11, 2021
Copy link
Author

@damemi damemi left a comment

Choose a reason for hiding this comment

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

/cc @sttts
do we need a bz for this?

Copy link

@sttts sttts Mar 11, 2021

Choose a reason for hiding this comment

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

all the cases with true you can revert to be just server.SetupSignalHandler().

Copy link

Choose a reason for hiding this comment

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

would avoid copying code. You can do:

func SetupSignalContextNotExiting() context.Context {
  return setupSignalContext(false)
}

func SetupSignalContext() context.Context {
  return setupSignalContext(true)
}

func setupSignalContext(exitOnSecondSignal bool) context.Context {
  ...
}

@openshift-ci-robot
Copy link

@damemi: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

@damemi
Copy link
Author

damemi commented Mar 11, 2021

@sttts thanks, I applied the changes you suggested in d096748. Could you double check that for me? If all looks good I'll squash

…ctions to ignore further signals

This patches the changes from openshift#558 to provide
these new functions without changing the behavior for other repos that depend on them, such
as library-go.
@damemi
Copy link
Author

damemi commented Mar 16, 2021

@sttts ping, squashed the commits here. Could you ptal?

@openshift-ci-robot
Copy link

@damemi: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

@sttts
Copy link

sttts commented Mar 22, 2021

/retest
/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 22, 2021
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damemi, sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 22, 2021
@stbenjam
Copy link
Member

Looks like this still has backports/unvalidated-commits - how does that label get removed?

@damemi
Copy link
Author

damemi commented Mar 22, 2021

It needs to be approved by a top-level approver I think? Maybe @smarterclayton could take a look?
/refresh

@soltysh soltysh removed the backports/unvalidated-commits Indicates that not all commits come to merged upstream PRs. label Mar 23, 2021
@soltysh
Copy link

soltysh commented Mar 23, 2021

It needs to be approved by a top-level approver I think? Maybe @smarterclayton could take a look?

That's a novel thing - updated, should merge shortly.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@stbenjam
Copy link
Member

/test k8s-e2e-gcp

@openshift-merge-robot openshift-merge-robot merged commit 52db538 into openshift:master Mar 24, 2021
@damemi
Copy link
Author

damemi commented May 11, 2021

/cherry-pick release-4.7

@openshift-cherrypick-robot

@damemi: new pull request created: #739

Details

In response to this:

/cherry-pick release-4.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. vendor-update Touching vendor dir or related files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants