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.

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

Delete keystone test helpers #339

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Delete keystone test helpers #339

merged 1 commit into from
Sep 26, 2023

Conversation

gibizer
Copy link
Contributor

@gibizer gibizer commented Sep 22, 2023 •
edited
Loading

This is necessary to remove a dependency cycle between keystone-operator and lib-common.

PR merging order:

@gibizer
Copy link
Contributor Author

gibizer commented Sep 22, 2023

/hold we need keystone-operator change and then we need to adapt all the service operator tests to use the helper from there

@gibizer gibizer requested a review from stuggi September 22, 2023 13:56
@gibizer gibizer force-pushed the keystone-test-helper branch from e1f4a23 to 9f56c50 Compare September 22, 2023 15:26
@gibizer
Copy link
Contributor Author

gibizer commented Sep 22, 2023

I need to split this into two PRs. The first does the necessary adaptation on the code that remains in lib-common so that it can be merged and allow merging the keystone-operator PR. Then the actual removal needs to be in a second PR after all the other operators are adapted to the move.

@gibizer gibizer force-pushed the keystone-test-helper branch from 9f56c50 to 81933d9 Compare September 22, 2023 15:49
@gibizer gibizer changed the title Move keystone test helpers to keystone-operator Delete keystone test helpers Sep 22, 2023
@gibizer
Copy link
Contributor Author

gibizer commented Sep 22, 2023

I need to split this into two PRs. The first does the necessary adaptation on the code that remains in lib-common so that it can be merged and allow merging the keystone-operator PR. Then the actual removal needs to be in a second PR after all the other operators are adapted to the move.

Done

@gibizer gibizer force-pushed the keystone-test-helper branch from 81933d9 to 262f3ed Compare September 22, 2023 16:15
@gibizer
Copy link
Contributor Author

gibizer commented Sep 22, 2023

/hold for the dependencies

@gibizer
Delete keystone CRD helpers and the KeystoneAPIFixture
657099e 
They have been moved to keystone-operator.
@gibizer gibizer force-pushed the keystone-test-helper branch from 262f3ed to 657099e Compare September 25, 2023 12:21
@gibizer gibizer mentioned this pull request Sep 25, 2023
Merged
@gibizer
Copy link
Contributor Author

gibizer commented Sep 25, 2023

/hold we need to adapt the service operators first

gibizer
gibizer commented Sep 26, 2023
View reviewed changes
modules/test/go.mod
github.com/onsi/gomega v1.27.10
github.com/openstack-k8s-operators/infra-operator/apis v0.1.1-0.20230920125017-2c76cd203b44
github.com/openstack-k8s-operators/keystone-operator/api v0.1.1-0.20230920085319-92ae0260bbf3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is our main goal here

stuggi
stuggi approved these changes Sep 26, 2023
View reviewed changes
Copy link
Contributor

@stuggi stuggi left a comment

Choose a reason for hiding this comment

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

/lgtm

@stuggi stuggi merged commit 1664bbc into openstack-k8s-operators:main Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuggi stuggi stuggi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gibizer @stuggi