You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/08/19 03:36:55 UTC

[GitHub] [pulsar] youzipi opened a new pull request, #17172: [cleanup][broker][Modernizer] fix violations in pulsar-broker

youzipi opened a new pull request, #17172:
URL: https://github.com/apache/pulsar/pull/17172

   Master Issue: #12271 #16991 
   
   ### Motivation
   
   Apply Maven Modernizer plugin to enforce we move away from legacy APIs.
   
   ### Modifications
   
   Add Maven Modernizer plugin in pulsar-broker module and fix violation.
   
   ### Verifying this change
   
   
   This change is already covered by existing tests, such as *(please describe tests)*.
   
   
   
   
   ### Does this pull request potentially affect one of the following parts:
   
   
     - Dependencies (does it add or upgrade a dependency): (no)
     - The public API: (no)
     - The schema: (no)
     - The default values of configurations: (no)
     - The wire protocol: (no)
     - The rest endpoints: (no)
     - The admin cli options: (no)
     - Anything that affects deployment: (no)
   
   ### Documentation
   
   - [x] `doc-not-needed` 
    


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] youzipi commented on pull request #17172: [cleanup][broker][Modernizer] fix violations in pulsar-broker

Posted by GitBox <gi...@apache.org>.
youzipi commented on PR #17172:
URL: https://github.com/apache/pulsar/pull/17172#issuecomment-1220697696

   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] youzipi commented on pull request #17172: [cleanup][broker][Modernizer] fix violations in pulsar-broker

Posted by GitBox <gi...@apache.org>.
youzipi commented on PR #17172:
URL: https://github.com/apache/pulsar/pull/17172#issuecomment-1225507442

   ping @codelipenghui 


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] youzipi commented on a diff in pull request #17172: [cleanup][broker][Modernizer] fix violations in pulsar-broker

Posted by GitBox <gi...@apache.org>.
youzipi commented on code in PR #17172:
URL: https://github.com/apache/pulsar/pull/17172#discussion_r952113910


##########
pulsar-broker/src/main/java/org/apache/pulsar/PulsarBrokerStarter.java:
##########
@@ -240,8 +240,8 @@ && isBlank(starterArguments.bookieConfigFile)) {
 
             // init bookie server
             if (starterArguments.runBookie) {
-                checkNotNull(bookieConfig, "No ServerConfiguration for Bookie");
-                checkNotNull(bookieStatsProvider, "No Stats Provider for Bookie");
+                Objects.requireNonNull(bookieConfig, "No ServerConfiguration for Bookie");

Review Comment:
   guava `chekNotNull` and jdk `Objects.requireNonNull` provide the same simple semantic:
   ```java
   if (obj == null)
       throw new NullPointerException(message);
   return obj;
   ```
   @hangc0276 



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] youzipi commented on pull request #17172: [cleanup][broker][Modernizer] fix violations in pulsar-broker

Posted by GitBox <gi...@apache.org>.
youzipi commented on PR #17172:
URL: https://github.com/apache/pulsar/pull/17172#issuecomment-1221208962

   @tisonkun @MarvinCai  @codelipenghui 


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] hangc0276 merged pull request #17172: [cleanup][broker][Modernizer] fix violations in pulsar-broker

Posted by GitBox <gi...@apache.org>.
hangc0276 merged PR #17172:
URL: https://github.com/apache/pulsar/pull/17172


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] tisonkun commented on a diff in pull request #17172: [cleanup][broker][Modernizer] fix violations in pulsar-broker

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17172:
URL: https://github.com/apache/pulsar/pull/17172#discussion_r952107099


##########
pulsar-broker/src/main/java/org/apache/pulsar/PulsarBrokerStarter.java:
##########
@@ -240,8 +240,8 @@ && isBlank(starterArguments.bookieConfigFile)) {
 
             // init bookie server
             if (starterArguments.runBookie) {
-                checkNotNull(bookieConfig, "No ServerConfiguration for Bookie");
-                checkNotNull(bookieStatsProvider, "No Stats Provider for Bookie");
+                Objects.requireNonNull(bookieConfig, "No ServerConfiguration for Bookie");

Review Comment:
   I think this decision is made in https://github.com/apache/pulsar/issues/12271 where we agree on enabling the Modernizer plugin and this is the default settings.
   
   Besides, Guava gradually deprecates `Lists.newArrayList` since JDK provides it out-of-the-box. This could be the same to `chekNotNull` to `Objects.requireNonNull`.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] hangc0276 commented on a diff in pull request #17172: [cleanup][broker][Modernizer] fix violations in pulsar-broker

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on code in PR #17172:
URL: https://github.com/apache/pulsar/pull/17172#discussion_r952104012


##########
pulsar-broker/src/main/java/org/apache/pulsar/PulsarBrokerStarter.java:
##########
@@ -240,8 +240,8 @@ && isBlank(starterArguments.bookieConfigFile)) {
 
             // init bookie server
             if (starterArguments.runBookie) {
-                checkNotNull(bookieConfig, "No ServerConfiguration for Bookie");
-                checkNotNull(bookieStatsProvider, "No Stats Provider for Bookie");
+                Objects.requireNonNull(bookieConfig, "No ServerConfiguration for Bookie");

Review Comment:
   Why change `chekNotNull` to `Objects.requireNonNull` ?



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org