Skip to content

[ROCKETMQ-32] Improve concision - Reuse local variable 'brokerAddrs' in RouteInfoManager.getSystemTopicList method#8

Closed
zhaozhanh wants to merge 25 commits into
apache:masterfrom
zhaozhanh:master
Closed

[ROCKETMQ-32] Improve concision - Reuse local variable 'brokerAddrs' in RouteInfoManager.getSystemTopicList method#8
zhaozhanh wants to merge 25 commits into
apache:masterfrom
zhaozhanh:master

Conversation

@zhaozhanh

Copy link
Copy Markdown

reuse brokerAddrs, or change like following:
if (bd.getBrokerAddrs() != null && !bd.getBrokerAddrs().isEmpty()) { Iterator<Long> it2 = bd.getBrokerAddrs().keySet().iterator(); topicList.setBrokerAddr(bd.getBrokerAddrs().get(it2.next())); break; }

@zhouxinyu

Copy link
Copy Markdown
Member

Hi @naughtybear,

Thanks your PR, could you abide our PR process(modify your PR subject and mention your jira address in the description), please follow this PR. #5

@zhaozhanh

Copy link
Copy Markdown
Author

Hi @zhouxinyu ,

Sorry.
I just start learning and wanna contributing, it's very nice of you to guide me.

Code concision improvement.
JIRA Issue: https://issues.apache.org/jira/browse/ROCKETMQ-11

@vongosling

Copy link
Copy Markdown
Member

please modify you PR subject and fix conflict problems.

@zhaozhanh

Copy link
Copy Markdown
Author

@vongosling

All done.

@vongosling

Copy link
Copy Markdown
Member

Subject still is "simplify if grammar "?please bind with JIRA. You can refer to other PR:-)

@zhaozhanh zhaozhanh changed the title simplify if grammar Improve concision - Reuse local variable 'brokerAddrs' in RouteInfoManager.getSystemTopicList method Dec 30, 2016
@vongosling

Copy link
Copy Markdown
Member

Could you polish again just like other PR. such as title start with JIRA issues. The following links may be helpful for you.

http://rocketmq.incubator.apache.org/docs/pull-request/
http://www.bigcloud.online/?p=280

@zhaozhanh

Copy link
Copy Markdown
Author

@vongosling
Hi,
two places still not align with the pr procedure.

  1. pull request title should be "[ROCKETMQ-11] Improve concision - Reuse local variable 'brokerAddrs' in RouteInfoManager.getSystemTopicList method [ROCKETMQ-32] Improve concision - Reuse local variable 'brokerAddrs' in RouteInfoManager.getSystemTopicList method #8"
  2. commit comment should be "[ROCKETMQ-11] Improve concision - Reuse local variable 'brokerAddrs' in RouteInfoManager.getSystemTopicList method, close [ROCKETMQ-32] Improve concision - Reuse local variable 'brokerAddrs' in RouteInfoManager.getSystemTopicList method #8"

Am I right? If so, I need start a new pull request and modify JIRA info to keep aligned.
Many thanks.

@vongosling

Copy link
Copy Markdown
Member

Yes, do not start a new pull request.Just modify your topic, start a JIRA and bind them :-)

@zhaozhanh zhaozhanh changed the title Improve concision - Reuse local variable 'brokerAddrs' in RouteInfoManager.getSystemTopicList method [ROCKETMQ-32] Improve concision - Reuse local variable 'brokerAddrs' in RouteInfoManager.getSystemTopicList method Jan 5, 2017
@zhaozhanh

Copy link
Copy Markdown
Author

@vongosling
Hi,
new JIRA Issue: https://issues.apache.org/jira/browse/ROCKETMQ-32

Is that be Ok?

@zhouxinyu

Copy link
Copy Markdown
Member

It seems ok, thanks @naughtybear .

Please @vongosling @lizhanhui help review it.

@zhaozhanh

Copy link
Copy Markdown
Author

@vongosling @lizhanhui
Please help review. Thanks.

while (it.hasNext()) {
BrokerData bd = brokerAddrTable.get(it.next());
HashMap<Long, String> brokerAddrs = bd.getBrokerAddrs();
if (bd.getBrokerAddrs() != null && !bd.getBrokerAddrs().isEmpty()) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

code smell

@vongosling

Copy link
Copy Markdown
Member

alright

@asfgit asfgit closed this in 12b6b9e Jan 16, 2017
lizhanhui pushed a commit to lizhanhui/rocketmq that referenced this pull request Jun 25, 2019
duhenglucky pushed a commit that referenced this pull request Oct 28, 2019
JiaMingLiu93 pushed a commit to JiaMingLiu93/rocketmq that referenced this pull request May 28, 2020
JiaMingLiu93 pushed a commit to JiaMingLiu93/rocketmq that referenced this pull request May 28, 2020
pingww pushed a commit that referenced this pull request Aug 26, 2022
…is not exist

[ISSUE #8] fix rocketmq-mqtt check style error and .travis.yaml file is not exist
pulllock pushed a commit to pulllock/rocketmq that referenced this pull request Oct 19, 2023
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.

8 participants