Re: [PR] KAFKA-16643 Fix chaos modifier [kafka]
chia7712 commented on PR #15890: URL: https://github.com/apache/kafka/pull/15890#issuecomment-2163526554 @gongxuanzhang please fix the build error -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16643 Fix chaos modifier [kafka]
chia7712 commented on PR #15890: URL: https://github.com/apache/kafka/pull/15890#issuecomment-2162051279 @gongxuanzhang could you fix the conflicts? Also, please revert the unrelated changes. thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16643 Fix chaos modifier [kafka]
chia7712 commented on PR #15890: URL: https://github.com/apache/kafka/pull/15890#issuecomment-2103767353 > But I'm confused. Is there anything I can do about it @dongjinleekr had filed a PR (#10428) to be the start. However, the PR gets conflicts now, and not sure whether @dongjinleekr has free cycles to address it. You can file a new PR to address that. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16643 Fix chaos modifier [kafka]
gongxuanzhang commented on PR #15890: URL: https://github.com/apache/kafka/pull/15890#issuecomment-2103743672 @chia7712 get it! But I'm confused. Is there anything I can do about it -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16643 Fix chaos modifier [kafka]
chia7712 commented on PR #15890: URL: https://github.com/apache/kafka/pull/15890#issuecomment-2103727012 @gongxuanzhang This is still a huge patch, which contains a log of changes across all modules. This is a important improvement which can impact all Kafka developers, and so we should separate it to small PRs. Personally, the first patch should include 1) new checkstyle and 2) auto formatter ( see https://issues.apache.org/jira/browse/KAFKA-12572) After first patch gets merged, we can apply the rule to all modules "one by one". Yes, it needs many PRs but they make reviews workable. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16643 Fix chaos modifier [kafka]
gongxuanzhang commented on PR #15890: URL: https://github.com/apache/kafka/pull/15890#issuecomment-2103687645 @chia7712 I add rule into checkstyle.xml you can run ``` ./gradlew checkstyleMain checkstyleTest --continue ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16643 Fix chaos modifier [kafka]
gongxuanzhang commented on PR #15890: URL: https://github.com/apache/kafka/pull/15890#issuecomment-2100450694 @chia7712 I will resubmit my PR, introduce the new checkstyle and re-modify the PR according to the module. Thank you for your patience and best wishes -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16643 Fix chaos modifier [kafka]
chia7712 commented on PR #15890: URL: https://github.com/apache/kafka/pull/15890#issuecomment-2100256001 > In this PR I have followed the checkstyle ModifierOrder rule and passed the self-test. We need the checkstyle first, because it is hard to review those huge changes by human eyes > Thank you very much for your reply and I hope I can make some contribution to kafka Welcome to Kafka community. Please try to add new checkstyle import rules to small module. I'd like to review them for you -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16643 Fix chaos modifier [kafka]
gongxuanzhang commented on PR #15890: URL: https://github.com/apache/kafka/pull/15890#issuecomment-2100235766 @chia7712 Yes i do . I think these changes, though numerous, are still simple changes. Even with the introduction of new checkstyle file , the style we pursue is always "unity". In this PR I have followed the checkstyle ModifierOrder rule and passed the self-test. Thank you very much for your reply and I hope I can make some contribution to kafka -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16643 Fix chaos modifier [kafka]
chia7712 commented on PR #15890: URL: https://github.com/apache/kafka/pull/15890#issuecomment-2100176348 @gongxuanzhang thanks for this patch. Could you take a look at https://issues.apache.org/jira/browse/KAFKA-10787? Personally, we should introduce the new checkstyle file. And then we apply it by module to reduce the patch size. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
