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 an interface in fileconsumer instead #25166

Closed

Conversation

VihasMakwana
Copy link
Contributor

Description: Create a Manager Interface for fileconsumer.
This would result in a cleaner and more readable code for threadpool PR.
Going with the interface is much better and it will keep the threadpool wholly isolated.

@djaglowski, I have the threadpool changes ready to be pushed. I need to know your POV for my approach to moving towards interfaces.

@VihasMakwana
Copy link
Contributor Author

VihasMakwana commented Aug 11, 2023

@djaglowski
If we keep only one Manager for both threadpool and the normal version for fileconsumer, it would be a mess with multiple if-else blocks everywhere in the code. Old code will be affected as well. It's better to create another Manager for threadpool which implements the ManagerInterface and keep it in a separate file.

@VihasMakwana
Copy link
Contributor Author

@atoulme just to give you some context around the changes:

@VihasMakwana VihasMakwana changed the title chore: move towards interface Add an interface in fileconsumer instead Aug 11, 2023
@djaglowski
Copy link
Member

Will this result in duplicated code between two different implementations of the manager?

Ultimately, I don't think I can evaluate this change independently of how it is used.

@VihasMakwana
Copy link
Contributor Author

Not needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants