Skip to content

Conversation

@joelanford
Copy link
Member

This PR reduces the cyclomatic complexity of the reconcile method of the operator controller from 8 to 5 (as measured by gocyclo), mainly by factoring out the bundle entity lookup from the solution into a separate function.

It also ensures all code paths set the Ready condition, which required two new reasons (BundleLookupFailed and BundleDeploymentFailed)

This is built on top of #115, so
/hold
until that has merged and we've rebased this one.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 3, 2023
@joelanford joelanford force-pushed the reduce-reconcile-complexity branch from 98a258c to 6ad5c2b Compare February 3, 2023 15:58
- reduce cyclomatic complexity from 8 to 5
- make it more obvious when and how conditions are set when various
  errors occur
- when errors occur: always set ready condition, always return the error
@joelanford joelanford force-pushed the reduce-reconcile-complexity branch from 6ad5c2b to 9453ff6 Compare February 7, 2023 04:30
@joelanford
Copy link
Member Author

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 7, 2023
Copy link
Contributor

@tmshort tmshort left a comment

Choose a reason for hiding this comment

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

/lgtm

Removing the loop, and doing a complete SetStatusCondition() in every error location makes this a lot clearer.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 7, 2023
@joelanford joelanford merged commit 520219c into operator-framework:main Feb 7, 2023
@joelanford joelanford deleted the reduce-reconcile-complexity branch February 7, 2023 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants