Skip to content

a slight change.#15812

Closed
gongxuanzhang wants to merge 0 commit intoapache:trunkfrom
gongxuanzhang:trunk
Closed

a slight change.#15812
gongxuanzhang wants to merge 0 commit intoapache:trunkfrom
gongxuanzhang:trunk

Conversation

@gongxuanzhang
Copy link
Copy Markdown
Contributor

The modifiers should be in uniform order

@gharris1727
Copy link
Copy Markdown
Contributor

Hi @gongxuanzhang and thank you for the contribution!

This is actually an issue in a lot of places, I checked with this checkstyle.xml rule:

    <module name="ModifierOrder"/>

and ./gradlew checkstyleMain checkstyleTest --continue.

While fixing this in one place is good, it would make sense to try and fix this everywhere if we decide to address it at all. I've created https://issues.apache.org/jira/browse/KAFKA-16643 for this, and you can take this on if you're interested. If so, please see the contributing guide: https://kafka.apache.org/contributing.html and join the mailing list and JIRA.

module violations
:streams:upgrade-system-tests-24:checkstyleTest 1
:streams:upgrade-system-tests-22:checkstyleTest 1
:streams:upgrade-system-tests-23:checkstyleTest 1
:streams:upgrade-system-tests-26:checkstyleTest 1
:streams:upgrade-system-tests-28:checkstyleTest 1
:streams:upgrade-system-tests-30:checkstyleTest 1
:streams:upgrade-system-tests-25:checkstyleTest 1
:streams:upgrade-system-tests-31:checkstyleTest 1
:streams:upgrade-system-tests-27:checkstyleTest 1
:streams:upgrade-system-tests-32:checkstyleTest 1
:streams:upgrade-system-tests-36:checkstyleTest 1
:streams:upgrade-system-tests-35:checkstyleTest 1
:streams:upgrade-system-tests-34:checkstyleTest 1
:streams:upgrade-system-tests-33:checkstyleTest 1
:streams:upgrade-system-tests-37:checkstyleTest 1
:generator:checkstyleMain 2
:connect:mirror-client:checkstyleMain 1
:connect:api:checkstyleMain 1
:connect:json:checkstyleMain 1
:server-common:checkstyleMain 10
:raft:checkstyleMain 11
:storage:checkstyleMain 2
:trogdor:checkstyleMain 17
:server:checkstyleMain 122
:connect:mirror:checkstyleMain 3
:connect:test-plugins:checkstyleMain 2
:tools:checkstyleMain 9
:storage:storage-api:checkstyleMain 20
:streams:examples:checkstyleMain 5
:streams:test-utils:checkstyleMain 2
:group-coordinator:checkstyleMain 46
:metadata:checkstyleMain 72
:raft:checkstyleTest 5
:connect:runtime:checkstyleMain 2
:group-coordinator:checkstyleTest 10
:server-common:checkstyleTest 4
:trogdor:checkstyleTest 3
:metadata:checkstyleTest 73
:streams:test-utils:checkstyleTest 12
:streams:checkstyleMain 57
:clients:checkstyleTest 87
:clients:checkstyleMain 122
:core:checkstyleMain 1
:shell:checkstyleMain 10
:core:checkstyleTest 12
:shell:checkstyleTest 2
:jmh-benchmarks:checkstyleMain 1
:connect:mirror:checkstyleTest 4
:connect:runtime:checkstyleTest 15
:streams:checkstyleTest 201

@gongxuanzhang
Copy link
Copy Markdown
Contributor Author

Hi @gongxuanzhang and thank you for the contribution!

This is actually an issue in a lot of places, I checked with this checkstyle.xml rule:

    <module name="ModifierOrder"/>

and ./gradlew checkstyleMain checkstyleTest --continue.

While fixing this in one place is good, it would make sense to try and fix this everywhere if we decide to address it at all. I've created https://issues.apache.org/jira/browse/KAFKA-16643 for this, and you can take this on if you're interested. If so, please see the contributing guide: https://kafka.apache.org/contributing.html and join the mailing list and JIRA.

module violations
:streams:upgrade-system-tests-24:checkstyleTest 1
:streams:upgrade-system-tests-22:checkstyleTest 1
:streams:upgrade-system-tests-23:checkstyleTest 1
:streams:upgrade-system-tests-26:checkstyleTest 1
:streams:upgrade-system-tests-28:checkstyleTest 1
:streams:upgrade-system-tests-30:checkstyleTest 1
:streams:upgrade-system-tests-25:checkstyleTest 1
:streams:upgrade-system-tests-31:checkstyleTest 1
:streams:upgrade-system-tests-27:checkstyleTest 1
:streams:upgrade-system-tests-32:checkstyleTest 1
:streams:upgrade-system-tests-36:checkstyleTest 1
:streams:upgrade-system-tests-35:checkstyleTest 1
:streams:upgrade-system-tests-34:checkstyleTest 1
:streams:upgrade-system-tests-33:checkstyleTest 1
:streams:upgrade-system-tests-37:checkstyleTest 1
:generator:checkstyleMain 2
:connect:mirror-client:checkstyleMain 1
:connect:api:checkstyleMain 1
:connect:json:checkstyleMain 1
:server-common:checkstyleMain 10
:raft:checkstyleMain 11
:storage:checkstyleMain 2
:trogdor:checkstyleMain 17
:server:checkstyleMain 122
:connect:mirror:checkstyleMain 3
:connect:test-plugins:checkstyleMain 2
:tools:checkstyleMain 9
:storage:storage-api:checkstyleMain 20
:streams:examples:checkstyleMain 5
:streams:test-utils:checkstyleMain 2
:group-coordinator:checkstyleMain 46
:metadata:checkstyleMain 72
:raft:checkstyleTest 5
:connect:runtime:checkstyleMain 2
:group-coordinator:checkstyleTest 10
:server-common:checkstyleTest 4
:trogdor:checkstyleTest 3
:metadata:checkstyleTest 73
:streams:test-utils:checkstyleTest 12
:streams:checkstyleMain 57
:clients:checkstyleTest 87
:clients:checkstyleMain 122
:core:checkstyleMain 1
:shell:checkstyleMain 10
:core:checkstyleTest 12
:shell:checkstyleTest 2
:jmh-benchmarks:checkstyleMain 1
:connect:mirror:checkstyleTest 4
:connect:runtime:checkstyleTest 15
:streams:checkstyleTest 201

Thank you for your answer!
I will continue to follow up and contribute.

@gharris1727
Copy link
Copy Markdown
Contributor

@gongxuanzhang Do you want to merge this PR as-is, or do you want to add more fixes to this?

@gongxuanzhang
Copy link
Copy Markdown
Contributor Author

@gongxuanzhang Do you want to merge this PR as-is, or do you want to add more fixes to this?

I choose the latter and do more
thank you for your trust

@gongxuanzhang
Copy link
Copy Markdown
Contributor Author

@gongxuanzhang Do you want to merge this PR as-is, or do you want to add more fixes to this?

I will create new pr linked

@gongxuanzhang
Copy link
Copy Markdown
Contributor Author

@gharris1727 i request a jira acount link gongxuanzhang email: gongxuanzhangmelt@gmail.com
I need a confirmation. Can I get it from you

@gongxuanzhang
Copy link
Copy Markdown
Contributor Author

@gharris1727 new pr : #15890

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.

2 participants