Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Improvement] Introduce Maven Modernizer plugin to Pulsar #12271

Closed
20 of 24 tasks
MarvinCai opened this issue Oct 3, 2021 · 9 comments
Closed
20 of 24 tasks

[Improvement] Introduce Maven Modernizer plugin to Pulsar #12271

MarvinCai opened this issue Oct 3, 2021 · 9 comments
Assignees
Labels
good first issue Good for newcomers help wanted type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Comments

@MarvinCai
Copy link
Contributor

MarvinCai commented Oct 3, 2021

As discussed in dev mail list: https://lists.apache.org/thread.html/re99b342765b2cfb26f3da0eedfb78e7312de158bf6c7ed0085d32781%40%3Cdev.pulsar.apache.org%3E
We want to introduce Maven Modernizer plugin to Pulsar.
This plugin helps detecting uses of legacy APIs which can be replaced with modern APIs that are often more performant, safer, and idiomatic than the legacy equivalents.
We'll break down the task by sub-module so anyone interested can work on a sub-module independently.
We'll start with checking violation with Java 8. Acceptance criteria will be passing existing CI.
Help are welcome, please reply with module you want to work on so there won't be conflict. And if change for single module is too big, please break down pr to reasonable size, say < 100 change so it's easier to review.
Please don't close the master issue via comment.

<plugin>
    <groupId>org.gaul</groupId>
    <artifactId>modernizer-maven-plugin</artifactId>
    <configuration>
        <failOnViolations>true</failOnViolations>
        <javaVersion>8</javaVersion>
    </configuration>
    <executions>
        <execution>
            <id>modernizer</id>
            <goals>
                <goal>modernizer</goal>
            </goals>
        </execution>
    </executions>
</plugin>
  • managed-ledger
  • pulsar-common
  • pulsar-broker-common
  • pulsar-broker-auth-athenz
  • pulsar-broker-auth-sasl
  • pulsar-broker
  • pulsar-client-api
  • pulsar-client
  • pulsar-client-admin-api
  • pulsar-client-admin
  • pulsar-client-tools
  • pulsar-client-tools-test
  • pulsar-client-auth-athenz
  • pulsar-client-auth-sasl
  • pulsar-websocket
  • pulsar-proxy
  • pulsar-discovery-service
  • pulsar-zookeeper-utils
  • pulsar-testclient
  • pulsar-config-validation
  • pulsar-transaction
  • pulsar-functions
  • pulsar-metadata
  • pulsar-package-management
@MarvinCai MarvinCai added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Oct 3, 2021
@MarvinCai
Copy link
Contributor Author

MarvinCai commented Oct 3, 2021

Please tag with help-wanted & good-first-issue
I'll work on pulsar-common

@amarlearning
Copy link
Contributor

@MarvinCai @eolivelli I would like to work on pulsar-proxy 😄

codelipenghui pushed a commit that referenced this issue Oct 30, 2021
…ix violation. (#12270)

Master Issue: #12271

### Motivation
Apply Maven Modernizer plugin to enforce we move away from legacy APIs.

### Modifications
Add Maven Modernizer plugin in pulsar-common module and fix violation.

### Verifying this change

This change is already covered by existing tests, such as *(please describe tests)*.
eolivelli pushed a commit to eolivelli/pulsar that referenced this issue Nov 29, 2021
…ix violation. (apache#12270)

Master Issue: apache#12271

### Motivation
Apply Maven Modernizer plugin to enforce we move away from legacy APIs.

### Modifications
Add Maven Modernizer plugin in pulsar-common module and fix violation.

### Verifying this change

This change is already covered by existing tests, such as *(please describe tests)*.
@github-actions
Copy link

The issue had no activity for 30 days, mark with Stale label.

@youzipi
Copy link
Contributor

youzipi commented Aug 6, 2022

i will work on pulsar-transaction and pulsar-broker @MarvinCai

@tisonkun
Copy link
Member

tisonkun commented Aug 6, 2022

@MarvinCai you can create a subtask for pulsar-broker for @youzipi, ask them to comment there, and assign.

Although, pulsar-broker is a heated code path various patches modify its code. @youzipi you may find a way to separate it into more fine-grained patches to avoid conflict or contact with domain experts like @codelipenghui to cooperate on that module. I assume that applying this plugin is like a code style reformat among the whole module.

hangc0276 pushed a commit that referenced this issue Aug 24, 2022
Master Issue: #12271 #16991 

### Motivation

Apply Maven Modernizer plugin to enforce we move away from legacy APIs.

### Modifications

- set `failOnViolations`=true
- fix violations except `broker` and `client` packages
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this issue Aug 29, 2022
…#17172)

Master Issue: apache#12271 apache#16991 

### Motivation

Apply Maven Modernizer plugin to enforce we move away from legacy APIs.

### Modifications

- set `failOnViolations`=true
- fix violations except `broker` and `client` packages
@MarvinCai
Copy link
Contributor Author

@tisonkun seems we've enabled the plugin for all modules, can close this issue?

@tisonkun
Copy link
Member

tisonkun commented Sep 5, 2022

@tisonkun
Copy link
Member

tisonkun commented Sep 5, 2022

Concretely, we turn on the plugin for pulsar-broker, but ignore several patterns now for gradually handling them. I think @youzipi is actively working on this. We can help with reviewing.

Flaky tests hurt a lot :(

@tisonkun
Copy link
Member

@MarvinCai @codelipenghui @nodece should be closed by #17968.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

No branches or pull requests

5 participants