Skip to content

MINOR: Style fixes to KafkaRaftClient#10809

Merged
abbccdda merged 1 commit into
apache:trunkfrom
abbccdda:raft_fix
Jun 4, 2021
Merged

MINOR: Style fixes to KafkaRaftClient#10809
abbccdda merged 1 commit into
apache:trunkfrom
abbccdda:raft_fix

Conversation

@abbccdda

@abbccdda abbccdda commented Jun 3, 2021

Copy link
Copy Markdown

As title suggested, made some style fixes to the class KafkaRaftClient

Committer Checklist (excluded from commit message)

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

@showuon showuon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Thanks for the PR. Might need to merge trunk and trigger the jenkins build again since there are jdk 8 + scala 2.12 and jdk11 + scala 2.13 failed with:

Process 'Gradle Test Executor 127' finished with non-zero exit value 1

@abbccdda abbccdda merged commit 0358c21 into apache:trunk Jun 4, 2021
@ijuma

ijuma commented Jun 4, 2021

Copy link
Copy Markdown
Member

@abbccdda Committer approval is required for PRs to be merged. I don't see that here.

@abbccdda

abbccdda commented Jun 4, 2021

Copy link
Copy Markdown
Author

@ijuma That's a good call, should I revert it? It was just a styling change PR.

@ijuma

ijuma commented Jun 4, 2021

Copy link
Copy Markdown
Member

You don't have to revert it, but can you please get a committer approval?

@mumrah mumrah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, retroactively. Thanks @abbccdda!

@mumrah

mumrah commented Jun 4, 2021

Copy link
Copy Markdown
Member

@ijuma we should be able to configure the repo to require an approval from a committer for trunk PRs, have we looked into this? Similarly, we can set up CODEOWNERS for different areas of the project.

@ijuma

ijuma commented Jun 4, 2021

Copy link
Copy Markdown
Member

@mumrah We haven't and it would be good to do.

@mumrah

mumrah commented Jun 4, 2021

Copy link
Copy Markdown
Member

@ijuma i'll send something out on the devs list

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