Skip to content

Conversation

dugenkui03
Copy link
Contributor

What is the purpose of the change

#4130

Copy link
Contributor

@RongtongJin RongtongJin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/home/travis/build/apache/rocketmq/srvutil/src/main/java/org/apache/rocketmq/srvutil/FileWatchService.java:26:8: error: Unused import - java.security.NoSuchAlgorithmException.
Audit done.

Hi, CI not pass

@codecov-commenter
Copy link

codecov-commenter commented Apr 10, 2022

Codecov Report

Merging #4132 (049abc2) into develop (fd554ab) will increase coverage by 0.08%.
The diff coverage is 0.00%.

@@              Coverage Diff              @@
##             develop    #4132      +/-   ##
=============================================
+ Coverage      47.92%   48.01%   +0.08%     
- Complexity      5002     5011       +9     
=============================================
  Files            634      635       +1     
  Lines          42529    42496      -33     
  Branches        5573     5567       -6     
=============================================
+ Hits           20381    20403      +22     
+ Misses         19647    19604      -43     
+ Partials        2501     2489      -12     
Impacted Files Coverage Δ
...g/apache/rocketmq/client/impl/MQClientAPIImpl.java 13.98% <0.00%> (+0.11%) ⬆️
...ava/org/apache/rocketmq/test/util/VerifyUtils.java 46.26% <0.00%> (-2.99%) ⬇️
...ava/org/apache/rocketmq/filter/util/BitsArray.java 59.82% <0.00%> (-2.57%) ⬇️
...mq/client/impl/consumer/RebalanceLitePullImpl.java 72.05% <0.00%> (-1.48%) ⬇️
...e/rocketmq/client/impl/consumer/RebalanceImpl.java 43.75% <0.00%> (-1.18%) ⬇️
...ketmq/common/protocol/body/RegisterBrokerBody.java 83.69% <0.00%> (-1.09%) ⬇️
...he/rocketmq/client/impl/consumer/ProcessQueue.java 61.92% <0.00%> (-0.46%) ⬇️
...mq/client/impl/producer/DefaultMQProducerImpl.java 45.21% <0.00%> (-0.38%) ⬇️
...nt/impl/consumer/ConsumeMessageOrderlyService.java 50.00% <0.00%> (-0.36%) ⬇️
...ain/java/org/apache/rocketmq/store/MappedFile.java 51.40% <0.00%> (-0.36%) ⬇️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd554ab...049abc2. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 51.958% when pulling 049abc2 on dugenkui03:patch-012 into 8745d4c on apache:develop.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 51.958% when pulling 049abc2 on dugenkui03:patch-012 into 8745d4c on apache:develop.

@dugenkui03
Copy link
Contributor Author

dugenkui03 commented Apr 10, 2022

/home/travis/build/apache/rocketmq/srvutil/src/main/java/org/apache/rocketmq/srvutil/FileWatchService.java:26:8: error: Unused import - java.security.NoSuchAlgorithmException. Audit done.

Hi, CI not pass

Thanks for your review, PR is updated.

@dugenkui03 dugenkui03 requested a review from RongtongJin April 10, 2022 04:06
@tianliuliu
Copy link
Contributor

tianliuliu commented Apr 13, 2022

Some businesses that use the old MQ client have catch these exceptions. When they are deleted in this new client, the business compile will fail, resulting in incompatible client versions.

@dugenkui03
Copy link
Contributor Author

Some businesses that use the old MQ client have caught these exceptions. When they are deleted in this new client, the business compile will fail, resulting in incompatible client versions.

Yes, you are right. I think it is nessary to correct the method signature as soon as possible to avoid more technical debt. This pr should be a breaking change.

Describe in Chinese

你是对的。但是我认为仍然有必要修正这个错误以避免更多的技术债务,这对于相关方法算是 重大更新(beaking change)。

在提交此 PR 的过程中也考虑到了尽可能合理的向前兼容,例如 1处调用 永远不会抛出 2异常MQBrokerException、但鉴于该类实现的接口方法定义抛出了deletePlainAccessConfig、此处没有去掉MQBrokerException
image

@tianliuliu
Copy link
Contributor

Some businesses that use the old MQ client have caught these exceptions. When they are deleted in this new client, the business compile will fail, resulting in incompatible client versions.

Yes, you are right. I think it is nessary to correct the method signature as soon as possible to avoid more technical debt. This pr should be a breaking change.

Describe in Chinese

你是对的。但是我认为仍然有必要修正这个错误以避免更多的技术债务,这对于相关方法算是 重大更新(beaking change)。

在提交此 PR 的过程中也考虑到了尽可能合理的向前兼容,例如 1处调用 永远不会抛出 2异常MQBrokerException、但鉴于该类实现的接口方法定义抛出了deletePlainAccessConfig、此处没有去掉MQBrokerExceptionimage

link @RongtongJin

@francisoliverlee francisoliverlee added this to the 4.9.4 milestone Apr 28, 2022
@francisoliverlee francisoliverlee merged commit 2d66e95 into apache:develop Apr 28, 2022
GenerousMan pushed a commit to GenerousMan/rocketmq that referenced this pull request Aug 12, 2022
[ISSUE apache#4130] Remove the exception which will never be thrown by method from method signature
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants