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

Add AdditionalFilters field to cindy spec [RHCLOUD-28098] #852

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

gburges
Copy link
Contributor

@gburges gburges commented Sep 7, 2023

DO NOT MERGE UNTIL CYNDI RELEASES NEW VERSION

@gburges gburges requested a review from psav September 7, 2023 14:33
Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

The Seal of Approval

@gburges
Copy link
Contributor Author

gburges commented Sep 7, 2023

/retest

Copy link
Collaborator

@psav psav left a comment

Choose a reason for hiding this comment

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

I'm expecting to see a cyndi.go change here, without it nothing will happen, the new spec will be accepted, but it won't go anywhere. We should also add something to the tests - I think there is a test for cyndipipeline that ensures this field ends up in the right place.

@gburges gburges force-pushed the cindystuff branch 3 times, most recently from 13ff144 to ad06db2 Compare September 8, 2023 13:52
Copy link
Collaborator

@psav psav left a comment

Choose a reason for hiding this comment

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

The test here only tests that the new field is acceptable - it doesn't actually assert that those fields land in the cyndipipeline resource

@gburges
Copy link
Contributor Author

gburges commented Sep 8, 2023

/retest

@gburges gburges force-pushed the cindystuff branch 6 times, most recently from 3e70dfe to f84c53d Compare September 12, 2023 11:48
@gburges gburges requested a review from psav September 12, 2023 12:36
@psav
Copy link
Collaborator

psav commented Sep 19, 2023

Need a rebase @gburges but, where are we on the cyndi release?

@gburges
Copy link
Contributor Author

gburges commented Sep 20, 2023

Need a rebase @gburges but, where are we on the cyndi release?

rebased. Cyndi .12 got released already

@psav
Copy link
Collaborator

psav commented Oct 9, 2023

Can you rebase and fix conflicts please?

@psav psav merged commit 2d7cd0c into RedHatInsights:master Oct 10, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants