Skip to content

Conversation

@ewolinetz
Copy link
Contributor

will need to rebase after #121 merges in

Replaces #118

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 12, 2019
@ewolinetz
Copy link
Contributor Author

/test e2e-aws

@ewolinetz
Copy link
Contributor Author

/hold
putting on a hold so i can squash before merge

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 16, 2019
@ewolinetz ewolinetz requested review from jcantrill and josefkarasek and removed request for squat April 17, 2019 14:23
Copy link
Contributor

@jcantrill jcantrill left a comment

Choose a reason for hiding this comment

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

Generally lgtm. Only question I have is about files that seem to be duplicates which I assume are part of stubbing the new SDK. Should we remove them? I would be fine with a follow on clean up PR if it made sense.

monitoringv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
)

// Change below variables to serve metrics on different host or port.
Copy link
Contributor

Choose a reason for hiding this comment

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

Artifact from stubbing the code? Do we need to act upon this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is from the code generation. i don't think we need to act upon it unless we want to use a different port


func init() {
// Register the types with the Scheme so the components can map objects to GroupVersionKinds and back
AddToSchemes = append(AddToSchemes, v1.SchemeBuilder.AddToScheme)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this allow us to watch multiple kinds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we would do that in the controller, but yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ewolinetz
Copy link
Contributor Author

I want to wait and cherry pick #120 onto this

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 2, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 2, 2019
@josefkarasek josefkarasek self-requested a review May 3, 2019 12:51
@ewolinetz
Copy link
Contributor Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 3, 2019
@josefkarasek
Copy link

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 3, 2019
@openshift-merge-robot openshift-merge-robot merged commit cc59318 into openshift:master-post-release May 3, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ewolinetz, josefkarasek

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [ewolinetz,josefkarasek]

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

@richm
Copy link
Contributor

richm commented May 3, 2019

/cherrypick master-post-release

@openshift-cherrypick-robot

@richm: base branch (master-post-release) needs to differ from target branch (master-post-release)

In response to this:

/cherrypick master-post-release

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.

@ewolinetz ewolinetz deleted the sdk_bump branch December 1, 2020 17:11
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. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants