Skip to content

Make status handlers standalone helper funcs #309

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
Merged
openshift-merge-robot merged 1 commit into from
Oct 9, 2019
Merged

Make status handlers standalone helper funcs #309

openshift-merge-robot merged 1 commit into from
Oct 9, 2019

Conversation

@benjaminapetersen
Copy link
Contributor

@benjaminapetersen benjaminapetersen commented Oct 7, 2019 •
edited
Loading

Make status handlers standalone helper funcs that can be used across controllers

This makes them useful now that we want to break off into separate controllers. See:

/assign @jhadvig @spadgett

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 7, 2019
jhadvig
jhadvig reviewed Oct 8, 2019
View reviewed changes
Copy link
Member

@jhadvig jhadvig left a comment

Choose a reason for hiding this comment

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

Two small nits otherwise this is precisely what we need 👍

pkg/console/operator/operator.go Outdated
klog.V(4).Infoln("console is in an unmanaged state.")
if !reflect.DeepEqual(updatedStatus, configs.Operator) {
c.SyncStatus(updatedStatus)

Copy link
Member

@jhadvig jhadvig Oct 8, 2019

Choose a reason for hiding this comment

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

nit: extra line

Copy link
Contributor Author

@benjaminapetersen benjaminapetersen Oct 8, 2019

Choose a reason for hiding this comment

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

will do.

pkg/console/operator/sync_v400.go Outdated
"fmt"
"os"

"github.com/openshift/console-operator/pkg/console/status"
Copy link
Member

@jhadvig jhadvig Oct 8, 2019

Choose a reason for hiding this comment

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

nit: lets put this to the rest of the openshift pkgs

Copy link
Contributor Author

@benjaminapetersen benjaminapetersen Oct 8, 2019

Choose a reason for hiding this comment

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

thx!

@jhadvig jhadvig mentioned this pull request Oct 8, 2019
Merged
@benjaminapetersen
Copy link
Contributor Author

benjaminapetersen commented Oct 8, 2019

Updated!

@benjaminapetersen
Copy link
Contributor Author

benjaminapetersen commented Oct 8, 2019

/retest

jhadvig
jhadvig approved these changes Oct 8, 2019
View reviewed changes
Copy link
Member

@jhadvig jhadvig left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 8, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 8, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benjaminapetersen, jhadvig

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

@benjaminapetersen
Copy link
Contributor Author

benjaminapetersen commented Oct 8, 2019

/retest

level=fatal msg="failed to initialize the cluster: Cluster operator image-registry is still updating"

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 8, 2019

/retest

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

2 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 8, 2019

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 8, 2019

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 28816d4 into openshift:master Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhadvig jhadvig jhadvig approved these changes

@spadgett spadgett Awaiting requested review from spadgett

Assignees

@jhadvig jhadvig

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/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.

5 participants

@benjaminapetersen @openshift-ci-robot @openshift-bot @jhadvig @openshift-merge-robot