-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Comments
Please tag with help-wanted & good-first-issue |
@MarvinCai @eolivelli I would like to work on |
…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)*.
…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)*.
The issue had no activity for 30 days, mark with Stale label. |
i will work on pulsar-transaction and pulsar-broker @MarvinCai |
@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. |
…#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
@tisonkun seems we've enabled the plugin for all modules, can close this issue? |
@MarvinCai No. There're still several dangling issues: |
Concretely, we turn on the plugin for Flaky tests hurt a lot :( |
@MarvinCai @codelipenghui @nodece should be closed by #17968. |
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.
The text was updated successfully, but these errors were encountered: