You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by lizhanhui <gi...@git.apache.org> on 2017/03/16 04:44:53 UTC

[GitHub] incubator-rocketmq pull request #80: [ROCKETMQ-114] Add javadoc to codebase

GitHub user lizhanhui opened a pull request:

    https://github.com/apache/incubator-rocketmq/pull/80

    [ROCKETMQ-114] Add javadoc to codebase

    Add javadoc to codebase

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/lizhanhui/incubator-rocketmq ROCKETMQ-114

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-rocketmq/pull/80.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #80
    
----
commit ef09bcc13fb9fee71255f78568e5c59fd252064b
Author: Zhanhui Li <li...@apache.org>
Date:   2017-03-06T09:46:53Z

    Add doc

commit 678ea43fa77ebe213ea4f4c76fc3a3693dd44d2f
Author: Zhanhui Li <li...@apache.org>
Date:   2017-03-10T10:26:01Z

    Add javadoc.

commit 5662cd46a46d0e8c7dacb5ce4be3beecefca78f2
Author: Li Zhanhui <li...@apache.org>
Date:   2017-03-16T03:24:27Z

    Add javadoc to send message processor.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #80: [ROCKETMQ-114] Add javadoc to codebase

Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/80
  
    What's wrong with this PR. if no other polish, suggest close it:-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #80: [ROCKETMQ-114] Add javadoc to codebase

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/80
  
    
    [![Coverage Status](https://coveralls.io/builds/10619410/badge)](https://coveralls.io/builds/10619410)
    
    Coverage increased (+0.2%) to 31.057% when pulling **5662cd46a46d0e8c7dacb5ce4be3beecefca78f2 on lizhanhui:ROCKETMQ-114** into **6b7d206f09b928ea58fb3a62413a053f008b8c1c on apache:develop**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #80: [ROCKETMQ-114] Add javadoc to codebase

Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui closed the pull request at:

    https://github.com/apache/incubator-rocketmq/pull/80


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #80: [ROCKETMQ-114] Add javadoc to codebase

Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/80#discussion_r114733797
  
    --- Diff: broker/src/main/java/org/apache/rocketmq/broker/processor/SendMessageProcessor.java ---
    @@ -112,18 +156,21 @@ private RemotingCommand consumerSendMsgBack(final ChannelHandlerContext ctx, fin
                 return response;
             }
     
    +        // Make sure message store of the broker is writable.
             if (!PermName.isWriteable(this.brokerController.getBrokerConfig().getBrokerPermission())) {
                 response.setCode(ResponseCode.NO_PERMISSION);
                 response.setRemark("the broker[" + this.brokerController.getBrokerConfig().getBrokerIP1() + "] sending message is forbidden");
                 return response;
             }
     
    +        // TODO: we should warn client here because OP may carelessly get things wrong here.
    --- End diff --
    
    I think this needs another JIRA issue too.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #80: [ROCKETMQ-114] Add javadoc to codebase

Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/80
  
    Let's close this PR for now and merge parts of docs which helps.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #80: [ROCKETMQ-114] Add javadoc to codebase

Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/80#discussion_r114733685
  
    --- Diff: broker/src/main/java/org/apache/rocketmq/broker/BrokerStartup.java ---
    @@ -95,52 +141,65 @@ public static BrokerController createBrokerController(String[] args) {
                 final BrokerConfig brokerConfig = new BrokerConfig();
                 final NettyServerConfig nettyServerConfig = new NettyServerConfig();
                 final NettyClientConfig nettyClientConfig = new NettyClientConfig();
    -            nettyServerConfig.setListenPort(10911);
    +            nettyServerConfig.setListenPort(10911); // Set default broker listen port
                 final MessageStoreConfig messageStoreConfig = new MessageStoreConfig();
     
                 if (BrokerRole.SLAVE == messageStoreConfig.getBrokerRole()) {
    +
    +                // Use a conservative value for slave brokers so that consumer groups won't frequently change targeting
    +                // brokers when they are consuming messages at the boundary between being probably resident in physical
    +                // memory and swapped out.
                     int ratio = messageStoreConfig.getAccessMessageInMemoryMaxRatio() - 10;
                     messageStoreConfig.setAccessMessageInMemoryMaxRatio(ratio);
                 }
     
    -            if (commandLine.hasOption('p')) {
    +            // FIXME: log should not be null
    --- End diff --
    
    Creating a JIRA issue for this and having its number as a comment is a good practice.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #80: [ROCKETMQ-114] Add javadoc to codebase

Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/80
  
    @vongosling I suggest to create JIRA issues for all TODOs and FIXMEs here to make sure they are fixed someday.
    It's not critical though, if you want to merge, please go ahead ;)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---