[GitHub] [kafka] ijuma commented on a diff in pull request #13454: MINOR: move RecordReader from org.apache.kafka.tools to org.apache.co…
ijuma commented on code in PR #13454: URL: https://github.com/apache/kafka/pull/13454#discussion_r1156697756 ## clients/src/main/java/org/apache/kafka/common/RecordReader.java: ## @@ -14,10 +14,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.kafka.tools; +package org.apache.kafka.common; Review Comment: This would follow a similar pattern as the storage api classes, which seems reasonable to me. -- 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
[GitHub] [kafka] ijuma commented on a diff in pull request #13454: MINOR: move RecordReader from org.apache.kafka.tools to org.apache.co…
ijuma commented on code in PR #13454: URL: https://github.com/apache/kafka/pull/13454#discussion_r1157213420 ## clients/src/main/java/org/apache/kafka/common/RecordReader.java: ## @@ -14,10 +14,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.kafka.tools; +package org.apache.kafka.common; Review Comment: tools doesn't depend on core, only tool tests depend on core. Right? -- 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
[GitHub] [kafka] ijuma commented on a diff in pull request #13454: MINOR: move RecordReader from org.apache.kafka.tools to org.apache.co…
ijuma commented on code in PR #13454: URL: https://github.com/apache/kafka/pull/13454#discussion_r1157271849 ## clients/src/main/java/org/apache/kafka/common/RecordReader.java: ## @@ -14,10 +14,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.kafka.tools; +package org.apache.kafka.common; Review Comment: It's fine to use a separate module as you had. ## clients/src/main/java/org/apache/kafka/common/RecordReader.java: ## @@ -14,10 +14,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.kafka.tools; +package org.apache.kafka.common; Review Comment: It's fine to use a separate module as you have. -- 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
[GitHub] [kafka] ijuma commented on a diff in pull request #13454: MINOR: move RecordReader from org.apache.kafka.tools to org.apache.co…
ijuma commented on code in PR #13454: URL: https://github.com/apache/kafka/pull/13454#discussion_r1157341335 ## clients/src/main/java/org/apache/kafka/common/RecordReader.java: ## @@ -14,10 +14,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.kafka.tools; +package org.apache.kafka.common; Review Comment: Yes, makes sense. -- 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
