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]
