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

Refactor KafkaConsumerProcessor #871

Merged
merged 11 commits into from
Sep 29, 2023
Merged

Conversation

guillermocalvo
Copy link
Contributor

@guillermocalvo guillermocalvo commented Sep 18, 2023

I extracted some internal classes out of KafkaConsumerProcessor and split some methods into smaller ones.

@guillermocalvo guillermocalvo self-assigned this Sep 18, 2023
@guillermocalvo guillermocalvo marked this pull request as ready for review September 19, 2023 17:14
@sdelamo sdelamo added the type: improvement A minor improvement to an existing feature label Sep 22, 2023

private static final Logger LOG = LoggerFactory.getLogger(KafkaConsumerProcessor.class); // NOSONAR

final ConsumerInfo info;
Copy link
Contributor

Choose a reason for hiding this comment

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

should these be private?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's package-protected on purpose. Please note that ConsumerState itself is an @Internal, package-protected class.

final ConsumerInfo info;
final Consumer<?, ?> kafkaConsumer;
final Set<String> subscriptions;
Set<TopicPartition> assignments;
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be private

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Package-protected on purpose.

Copy link
Contributor

@jeremyg484 jeremyg484 left a comment

Choose a reason for hiding this comment

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

It looks like all of the new classes are missing @author and @since JavaDoc tags.

Disclaimer: some of these classes already existed,
but didn't have any authorship / version info available.
@sonarcloud
Copy link

sonarcloud bot commented Sep 26, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

83.2% 83.2% Coverage
0.0% 0.0% Duplication

@sdelamo sdelamo merged commit 73649c2 into master Sep 29, 2023
8 checks passed
@sdelamo sdelamo deleted the refactor-kafka-consumer-processor branch September 29, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement A minor improvement to an existing feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants