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

Add deleteConsumerGroups method #730

Merged
merged 4 commits into from
Jan 2, 2022
Merged

Add deleteConsumerGroups method #730

merged 4 commits into from
Jan 2, 2022

Conversation

keirlawson
Copy link
Contributor

No description provided.

@keirlawson
Copy link
Contributor Author

Failing due to mima, not sure how best to make this change in a way that doesn't trigger it?

@filosganga filosganga requested review from vlovgr and bplommer October 20, 2021 18:57
@LMnet
Copy link
Member

LMnet commented Oct 20, 2021 •
edited
Loading

You need to add an exclusion rule for a new method in the build.sbt

Overall I'm +1

vlovgr
vlovgr reviewed Oct 21, 2021
View reviewed changes
Copy link
Contributor

@vlovgr vlovgr left a comment

Choose a reason for hiding this comment

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

You can find the exclusion rule in the build logs and it needs to be added to mimaBinaryIssuesFilter.

modules/core/src/main/scala/fs2/kafka/KafkaAdminClient.scala Outdated
/**
* Delete consumer groups from the cluster.
*/
def deleteConsumerGroups(groupIds: List[String]): F[Unit]
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we change this to:

def deleteConsumerGroups[G[_]](groupIds: G[String])(
  implicit G: Foldable[G]
): F[Unit]

to match the other functions?

@keirlawson keirlawson requested a review from vlovgr October 21, 2021 08:31
@bplommer bplommer merged commit 48315ff into fd4s:series/2.x Jan 2, 2022
@keirlawson keirlawson deleted the admin-delete-cgs branch January 2, 2022 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bplommer bplommer bplommer approved these changes

@vlovgr vlovgr vlovgr approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@keirlawson @LMnet @vlovgr @bplommer