Skip to content

Fixes CAPI Status appearing in the UI #1265

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

Merged
merged 6 commits into from
Aug 11, 2022
Merged

Fixes CAPI Status appearing in the UI #1265

merged 6 commits into from
Aug 11, 2022

Conversation

foot
Copy link
Collaborator

@foot foot commented Aug 10, 2022

  • Needed to pipe the CAPIEnabled through our layers and layers of config
    a little bit more

Fixes #1227

- Needed to pipe the CAPIEnabled through our layers any layers of config
  a little bit more
@foot foot added the bug Something isn't working label Aug 10, 2022
@foot foot requested a review from saeedfazal August 11, 2022 06:51
@foot
Copy link
Collaborator Author

foot commented Aug 11, 2022

See smoke tests running in checks above ^

Copy link
Contributor

@bigkevmcd bigkevmcd left a comment

Choose a reason for hiding this comment

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

Looks ok to me, I am not sure that the func opts need to be exported tho' as they aren't used outside of the package.

@foot
Copy link
Collaborator Author

foot commented Aug 11, 2022

Looks ok to me, I am not sure that the func opts need to be exported tho' as they aren't used outside of the package.

I would lean towards stripping a layer or two our of this whole config dance we have right now. Including the With* things. AFAIUI the main benefit to that pattern is providing a stable interface to a lib consumer, which we don't have.

@bigkevmcd
Copy link
Contributor

is providing a stable interface to a lib consumer, which we don't have.

Yes, we aren't using them outside the package, so other than the readability part, there's not currently a huge benefit.

They could be unexported...

@foot
Copy link
Collaborator Author

foot commented Aug 11, 2022

They could be unexported...

Or deleted and replaced with a struct? wdyt?

@foot foot merged commit 2ddc1ae into main Aug 11, 2022
@foot foot deleted the fixes-capi-status-in-ui branch August 11, 2022 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WGE UI has stopped rendering capi cluster status details
2 participants