Skip to content

MINOR: Rename AlterIsrManager to AlterPartitionManager#12089

Merged
hachikuji merged 5 commits intoapache:trunkfrom
hachikuji:rename-alter-isr-manager
Apr 26, 2022
Merged

MINOR: Rename AlterIsrManager to AlterPartitionManager#12089
hachikuji merged 5 commits intoapache:trunkfrom
hachikuji:rename-alter-isr-manager

Conversation

@hachikuji
Copy link
Copy Markdown
Contributor

Since we have changed the AlterIsr API to AlterPartition, it makes sense to rename AlterIsrManager as well.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Copy link
Copy Markdown
Member

@jsancio jsancio left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Copy Markdown
Member

@dengziming dengziming left a comment

Choose a reason for hiding this comment

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

Thank you for this PR @hachikuji , left one question.

Comment thread core/src/test/scala/unit/kafka/cluster/AbstractPartitionTest.scala Outdated
Copy link
Copy Markdown
Member

@dajac dajac left a comment

Choose a reason for hiding this comment

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

Renaming it makes sense. Left a small nit.

Comment thread core/src/main/scala/kafka/server/AlterPartitionManager.scala Outdated
@hachikuji
Copy link
Copy Markdown
Contributor Author

@dajac @dengziming Thanks for reviews. Pushed an update.

Copy link
Copy Markdown
Member

@dajac dajac left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@dengziming dengziming left a comment

Choose a reason for hiding this comment

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

LGTM, there are still 1 minor improvements: ApiKeysTest.testAlterIsrIsClusterAction.

Comment thread core/src/test/scala/unit/kafka/server/KafkaServerTest.scala
@hachikuji hachikuji merged commit f2a782a into apache:trunk Apr 26, 2022
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