Skip to content
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

Use hybrid controller instead of the pre-built version #214

Merged

Conversation

skattoju
Copy link
Contributor

@skattoju skattoju commented Apr 6, 2023

This is to update the makefile template so that when a hybrid helm operator is skaffolded the hybrid controller is used instead of the pre-built one supplied with operator sdk

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 6, 2023
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 6, 2023
@skattoju skattoju force-pushed the makefile_hybrid_controller branch 3 times, most recently from 97c8f44 to 8dbd443 Compare April 11, 2023 21:15
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 11, 2023
@skattoju skattoju marked this pull request as ready for review April 11, 2023 21:17
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 11, 2023
@skattoju skattoju force-pushed the makefile_hybrid_controller branch 2 times, most recently from cf0593f to 2aedeff Compare April 12, 2023 19:38
@skattoju
Copy link
Contributor Author

@camilamacedo86 @SimonBaeumer ptal 👀

@skattoju
Copy link
Contributor Author

I think requested changes have been made but varsha is off so could you please take a look @camilamacedo86 @SimonBaeumer 👀

Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

@skattoju Thanks for taking this on! These changes look good to me!

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 24, 2023
@everettraven
Copy link
Contributor

@skattoju It does look like @varshaprasad96 's concerns have been addressed. I'd be happy to merge this pending another approving review to make sure I didn't miss anything.

@oceanc80 or @theishshah would either of you mind taking a look?

@oceanc80
Copy link
Contributor

/lgtm

Copy link
Member

@varshaprasad96 varshaprasad96 left a comment

Choose a reason for hiding this comment

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

/lgtm

@varshaprasad96 varshaprasad96 merged commit 7eacb66 into operator-framework:main May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sdk area/testing lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants