lhotari commented on code in PR #2:
URL: https://github.com/apache/pulsar-java-contrib/pull/2#discussion_r1728491450


##########
.github/pull_request_template.md:
##########
@@ -0,0 +1,68 @@
+<!--
+### Contribution Checklist
+  
+  - PR title format should be *[type][component] summary*. For details, see 
*[Guideline - Pulsar PR Naming 
Convention](https://pulsar.apache.org/contribute/develop-semantic-title/)*. 
+
+  - Fill out the template below to describe the changes contributed by the 
pull request. That will give reviewers the context they need to do the review.
+  
+  - Each pull request should address only one issue, not mix up code from 
multiple issues.
+  
+  - Each commit in the pull request has a meaningful commit message
+
+  - Once all items of the checklist are addressed, remove the above text and 
this checklist, leaving only the filled out template below.
+-->
+
+<!-- Either this PR fixes an issue, -->
+
+Fixes #xyz
+
+<!-- or this PR is one task of an issue -->
+
+Main Issue: #xyz
+
+<!-- If the PR belongs to a PIP, please add the PIP link here -->
+
+### Motivation
+
+<!-- Explain here the context, and why you're making that change. What is the 
problem you're trying to solve. -->
+
+### Modifications
+
+<!-- Describe the modifications you've done. -->
+
+### Verifying this change
+
+- [ ] Make sure that the change passes the CI checks.
+
+*(Please pick either of the following options)*
+
+This change is a trivial rework / code cleanup without any test coverage.
+
+*(or)*
+
+This change is already covered by existing tests, such as *(please describe 
tests)*.
+
+*(or)*
+
+This change added tests and can be verified as follows:
+
+*(example:)*
+- *Added integration tests for end-to-end deployment with large payloads 
(10MB)*
+- *Extended integration test for recovery after broker failure*
+
+### Matching PR in forked repository
+
+PR in forked repository: <!-- ENTER URL HERE -->
+
+<!--
+After opening this PR, the build in apache/pulsar will fail and instructions 
will
+be provided for opening a PR in the PR author's forked repository.
+
+apache/pulsar pull requests should be first tested in your own fork since the 
+apache/pulsar CI based on GitHub Actions has constrained resources and quota.
+GitHub Actions provides separate quota for pull requests that are executed in 
+a forked repository.
+
+The tests will be run in the forked repository until all PR review comments 
have
+been handled, the tests pass and the PR is approved by a reviewer.
+-->

Review Comment:
   This isn't relevant in this repository.



##########
etc/checkstyle.xml:
##########


Review Comment:
   remove checkstyle completely when spotless is used. There's also another 
checkstyle.xml file in the PR so that's also odd if we were to continue to use 
checkstyle.



##########
src/main/resources/suppressions.xml:
##########


Review Comment:
   if spotless is used, checkstyle could be removed.



##########
pom.xml:
##########
@@ -0,0 +1,239 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"; 
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
https://maven.apache.org/xsd/maven-4.0.0.xsd";>
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache</groupId>
+        <artifactId>apache</artifactId>
+        <version>29</version>
+    </parent>
+    <version>1.0.0-SNAPSHOT</version>
+
+    <inceptionYear>2024</inceptionYear>
+
+    <artifactId>pulsar-java-contrib</artifactId>
+    <packaging>pom</packaging>
+    <name>Pulsar Java Contrib</name>
+
+    <properties>
+        <lombok.version>1.18.32</lombok.version>
+        <slf4j.version>2.0.13</slf4j.version>
+        <maven.compiler.release>17</maven.compiler.release>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        <pulsar.version>3.3.1</pulsar.version>
+        
<maven-checkstyle-plugin.version>3.4.0</maven-checkstyle-plugin.version>
+        <puppycrawl.checkstyle.version>8.45.1</puppycrawl.checkstyle.version>
+        <spotless-maven-plugin.version>2.43.0</spotless-maven-plugin.version>
+        <maven-compiler-plugin>3.13.0</maven-compiler-plugin>
+        <maven-wrapper-plugin.version>3.3.2</maven-wrapper-plugin.version>
+        <license-maven-plugin.version>4.5</license-maven-plugin.version>
+        <org.testing.version>7.10.2</org.testing.version>
+        <maven-compiler-plugin.version>3.13.0</maven-compiler-plugin.version>
+    </properties>
+
+    <modules>
+        <module>pulsar-client-common-contrib</module>
+        <module>pulsar-loadbalance-contrib</module>
+        <module>pulsar-interceptor-contrib</module>
+        <module>pulsar-connector-contrib</module>
+        <module>pulsar-function-contrib</module>
+        <module>pulsar-bookkeeper-contrib</module>
+        <module>pulsar-transaction-contrib</module>
+        <module>pulsar-metrics-contrib</module>
+        <module>pulsar-auth-contrib</module>
+    </modules>
+
+    <profiles>
+        <profile>
+            <id>default</id>
+            <activation>
+                <property>
+                    <name>!productized</name>
+                </property>
+            </activation>
+        </profile>
+
+        <profile>
+            <id>fullProfile</id>
+            <activation>
+                <property>
+                    <name>full</name>
+                </property>
+            </activation>
+        </profile>
+    </profiles>

Review Comment:
   These profiles don't seem to be used, so they can be removed.



##########
src/main/resources/checkstyle.xml:
##########


Review Comment:
   When spotless is used, shouldn't checkstyle be removed completely?



##########
README.md:
##########
@@ -1,2 +1,25 @@
-# pulsar-java-contrib
-Apache pulsar
+# Apache Pulsar Java Contrib
+
+Pulsar java contrib is to provide a non-core code maintenance repository to 
collect plugin implementations, personalized features, experimental features, 
and best practices from users.
+
+- [Plugin Contribution Guide](contributionGuides) lists the core interfaces in 
Pulsar that can be implemented by contributors, and provides implementation 
guidelines for each type of interface.
+
+- [Plugin Implementation List](contributedFeatures.md) lists the implemented 
plugins. Users can select the ones they need for reuse.
+
+- [Personalization Features](customizationFeatures.md) lists the customized 
features and experimental features that require modification to the Pulsar 
source code.
+
+- [Best Practices]([best-practice-blogs](best-practice-blogs)) lists the best 
practices for each function summarized by community contributions.
+  - [consume-best-practice.md](best-practice-blogs%2Fconsume-best-practice.md)
+
+This project follows the terms of **Apache License 2.0**.
+You can format the code by `mvn spotless:apply` and generate license headers 
by `mvn license:format`.
+Please note that the code formatted by Spotless may still not meet the 
formatting requirements. Please run `mvn checkstyle:check` for inspection.

Review Comment:
   This doesn't make much sense. I think it's better to remove checkstyle 
completely when spotless is used. `spotless:check` should be used to enforce 
the configured spotless style.
   It's necessary to get this right from the beginning, before accepting 
contributions. 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to