Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

Complete the ProducerIdManager component #372

Merged
merged 4 commits into from
Feb 19, 2021
Merged

Conversation

gaoran10
Copy link
Contributor

@gaoran10 gaoran10 commented Feb 8, 2021

Complete the ProducerIdManager component and add tests for it.

@jiazhai
Copy link
Contributor

jiazhai commented Feb 9, 2021

@BewareMyPower Would you please also help take a review?

@BewareMyPower BewareMyPower added the type/feature Indicates new functionality label Feb 9, 2021
@BewareMyPower
Copy link
Collaborator

The Codacy Static Code Analysis is weird.

Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes.

private int brokerId = 1;

Could you adjust the brokerId position and try again? If Codacy Static Code Analysis still failed, we'll just ignore it.

@jiazhai
Copy link
Contributor

jiazhai commented Feb 9, 2021

The Codacy Static Code Analysis is weird.

Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes.

private int brokerId = 1;

Could you adjust the brokerId position and try again? If Codacy Static Code Analysis still failed, we'll just ignore it.

@gaoran10 FYI

@gaoran10
Copy link
Contributor Author

gaoran10 commented Feb 9, 2021

I found some methods before the field declarations, such as getListeners() and getKafkaAdvertisedListeners(), maybe we could adjust their positions, or ignored this code analysis problem.

@BewareMyPower
Copy link
Collaborator

@gaoran10 Yeah, you can adjust their positions. When these methods were added, the Codacy analysis was not enabled. And it seems that Codacy does only check the modified sources so that other PRs passed the tests.

@gaoran10 gaoran10 mentioned this pull request Feb 18, 2021
21 tasks
@BewareMyPower BewareMyPower merged commit e567ee5 into master Feb 19, 2021
@BewareMyPower BewareMyPower deleted the producer-id-manager branch February 19, 2021 09:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/feature Indicates new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants