Re: The new branching model has been put into effect.
Don't worry. The latest release is right, contains everything in the release note. We just should do a duplex merge after the release, simple, right? On Wed, Aug 02, 2017 at 8:51am, Roman Shtykh < rsht...@yahoo.com.invalid [rsht...@yahoo.com.invalid] > wrote: Hi Yukon, Thanks a lot! There are two issues then. 1. The latest release is wrong and probably doesn't contain everything as advertised. Shall we cancel it and re-release?Do mentors consider it a problem? 2. How can we make sure that the next release is done correctly? Obviously, 'develop' branch introduces extra efforts for merging and release. It doesn't seem work well. -- Roman On Tuesday, August 1, 2017 6:02 PM, yukonwrote: Hi Roman, The last release didn't do a right merge between develop and master branch, so we saw a big commit gap. I have made a synchronization between these two branches just now. Regards On Tue, Aug 1, 2017 at 4:12 PM, Roman Shtykh wrote: > Hi Yukon, > Can you confirm the release was done correctly (with all modifications > incorporated, as advertised)? > -- Roman > > > On Friday, July 14, 2017 10:37 AM, yukon wrote: > > > Hi, > > > Thanks guys, actually the release manager will do a sync between `master` > and `develop`(duplex merge) when release a new version, so this diff only > will occur in the gap between two releases. > > But... Maybe there is something wrong, I will check it~ > > > Regards, > yukon > > On Thu, Jul 13, 2017 at 10:00 AM, Zhanhui Li wrote: > > > Agree, develop and master branches should not split. We need to fix this > > issue. > > > > > 在 2017年7月12日,下午9:23,Roman Shtykh 写道: > > > > > > Folks, > > > Please have a look at https://github.com/apache/ > incubator-rocketmq/tree/ > > develop > > > This branch is 82 commits ahead, 74 commits behind master. <-- how can > > this happen? considering we had a recent release. > > > Do we have the situation Justin described before -- "In project where > we > > had this split it just ends up being more work, people accidentally check > > into the wrong branch and I’ve see a lot of merge issues"? > > > > > > I'll say it again, but it is very inconvenient to use gitflow model on > > github (when on github you normally commit on master branch). > > > > > > -- Roman > > > > > > On Friday, March 17, 2017 11:41 AM, Justin Mclean < > > jus...@classsoftware.com> wrote: > > > > > > > > > Hi, > > > > > > I also don't see there's a real need to split master and develop. In > > project where we had this split it just ends up being more work, people > > accidentally check into the wrong branch and I’ve see a lot of merge > issues. > > > > > > That being said the project is free to select whatever model they want. > > > > > > Thanks, > > > Justin > > > > > > > > > >
[GitHub] incubator-rocketmq pull request #134: Rito - #44 - Extracting out duplicated...
Github user shroman commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130764893 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java --- @@ -1215,9 +1210,13 @@ public TopicRouteData getTopicRouteInfoFromNameServer(final String topic, final assert response != null; switch (response.getCode()) { case ResponseCode.TOPIC_NOT_EXIST: { -if (!topic.equals(MixAll.DEFAULT_TOPIC)) +if (allowTopicNotExist && !topic.equals(MixAll.DEFAULT_TOPIC)) { log.warn("get Topic [{}] RouteInfoFromNameServer is not exist value", topic); -break; +break; --- End diff -- well, I mean you can have one for `case ResponseCode.TOPIC_NOT_EXIST`, don't you? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #134: Rito - #44 - Extracting out duplicated...
Github user shroman commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130764799 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java --- @@ -1215,9 +1210,13 @@ public TopicRouteData getTopicRouteInfoFromNameServer(final String topic, final assert response != null; switch (response.getCode()) { case ResponseCode.TOPIC_NOT_EXIST: { -if (!topic.equals(MixAll.DEFAULT_TOPIC)) +if (allowTopicNotExist && !topic.equals(MixAll.DEFAULT_TOPIC)) { log.warn("get Topic [{}] RouteInfoFromNameServer is not exist value", topic); -break; +break; +} else { +// TODO LOG --- End diff -- ok then --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
Re: The new branching model has been put into effect.
Hi Yukon, Thanks a lot! There are two issues then. 1. The latest release is wrong and probably doesn't contain everything as advertised. Shall we cancel it and re-release?Do mentors consider it a problem? 2. How can we make sure that the next release is done correctly? Obviously, 'develop' branch introduces extra efforts for merging and release. It doesn't seem work well. -- Roman On Tuesday, August 1, 2017 6:02 PM, yukonwrote: Hi Roman, The last release didn't do a right merge between develop and master branch, so we saw a big commit gap. I have made a synchronization between these two branches just now. Regards On Tue, Aug 1, 2017 at 4:12 PM, Roman Shtykh wrote: > Hi Yukon, > Can you confirm the release was done correctly (with all modifications > incorporated, as advertised)? > -- Roman > > > On Friday, July 14, 2017 10:37 AM, yukon wrote: > > > Hi, > > > Thanks guys, actually the release manager will do a sync between `master` > and `develop`(duplex merge) when release a new version, so this diff only > will occur in the gap between two releases. > > But... Maybe there is something wrong, I will check it~ > > > Regards, > yukon > > On Thu, Jul 13, 2017 at 10:00 AM, Zhanhui Li wrote: > > > Agree, develop and master branches should not split. We need to fix this > > issue. > > > > > 在 2017年7月12日,下午9:23,Roman Shtykh 写道: > > > > > > Folks, > > > Please have a look at https://github.com/apache/ > incubator-rocketmq/tree/ > > develop > > > This branch is 82 commits ahead, 74 commits behind master. <-- how can > > this happen? considering we had a recent release. > > > Do we have the situation Justin described before -- "In project where > we > > had this split it just ends up being more work, people accidentally check > > into the wrong branch and I’ve see a lot of merge issues"? > > > > > > I'll say it again, but it is very inconvenient to use gitflow model on > > github (when on github you normally commit on master branch). > > > > > > -- Roman > > > > > > On Friday, March 17, 2017 11:41 AM, Justin Mclean < > > jus...@classsoftware.com> wrote: > > > > > > > > > Hi, > > > > > > I also don't see there's a real need to split master and develop. In > > project where we had this split it just ends up being more work, people > > accidentally check into the wrong branch and I’ve see a lot of merge > issues. > > > > > > That being said the project is free to select whatever model they want. > > > > > > Thanks, > > > Justin > > > > > > > > > >
[GitHub] incubator-rocketmq issue #134: Rito - #44 - Extracting out duplicated code f...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/134 [![Coverage Status](https://coveralls.io/builds/12638697/badge)](https://coveralls.io/builds/12638697) Coverage decreased (-0.3%) to 38.79% when pulling **28e14be3ea8fbe24814bcdebd61b36d94dd5b3b2 on Ritabrata-TW:master** into **d4149207e27ed3516f1f06407b55986790b806ae on apache:master**. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #134: Rito - #44 - Extracting out duplicated code f...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/134 [![Coverage Status](https://coveralls.io/builds/12638697/badge)](https://coveralls.io/builds/12638697) Coverage decreased (-0.3%) to 38.79% when pulling **28e14be3ea8fbe24814bcdebd61b36d94dd5b3b2 on Ritabrata-TW:master** into **d4149207e27ed3516f1f06407b55986790b806ae on apache:master**. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #134: Rito - #44 - Extracting out duplicated code f...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/134 [![Coverage Status](https://coveralls.io/builds/12638697/badge)](https://coveralls.io/builds/12638697) Coverage decreased (-0.3%) to 38.79% when pulling **28e14be3ea8fbe24814bcdebd61b36d94dd5b3b2 on Ritabrata-TW:master** into **d4149207e27ed3516f1f06407b55986790b806ae on apache:master**. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #134: Rito - #44 - Extracting out duplicated...
Github user Ritabrata-TW commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130596160 --- Diff: store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java --- @@ -717,22 +718,28 @@ public long getEarliestMessageTime(String topic, int queueId) { long minLogicOffset = logicQueue.getMinLogicOffset(); SelectMappedBufferResult result = logicQueue.getIndexBuffer(minLogicOffset / ConsumeQueue.CQ_STORE_UNIT_SIZE); -if (result != null) { -try { -final long phyOffset = result.getByteBuffer().getLong(); -final int size = result.getByteBuffer().getInt(); -long storeTime = this.getCommitLog().pickupStoreTimestamp(phyOffset, size); -return storeTime; -} catch (Exception e) { -} finally { -result.release(); -} -} +Long storeTime = getStoreTime(result); +return storeTime; --- End diff -- Yup. Made the change. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #134: Rito - #44 - Extracting out duplicated...
Github user Ritabrata-TW commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130595893 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java --- @@ -1215,9 +1210,13 @@ public TopicRouteData getTopicRouteInfoFromNameServer(final String topic, final assert response != null; switch (response.getCode()) { case ResponseCode.TOPIC_NOT_EXIST: { -if (!topic.equals(MixAll.DEFAULT_TOPIC)) +if (allowTopicNotExist && !topic.equals(MixAll.DEFAULT_TOPIC)) { log.warn("get Topic [{}] RouteInfoFromNameServer is not exist value", topic); -break; +break; +} else { +// TODO LOG --- End diff -- @shroman The TODO was already there. Not sure what to do about it right now. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #134: Rito - #44 - Extracting out duplicated...
Github user Ritabrata-TW commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130595647 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java --- @@ -1215,9 +1210,13 @@ public TopicRouteData getTopicRouteInfoFromNameServer(final String topic, final assert response != null; switch (response.getCode()) { case ResponseCode.TOPIC_NOT_EXIST: { -if (!topic.equals(MixAll.DEFAULT_TOPIC)) +if (allowTopicNotExist && !topic.equals(MixAll.DEFAULT_TOPIC)) { log.warn("get Topic [{}] RouteInfoFromNameServer is not exist value", topic); -break; +break; --- End diff -- @shroman We don't have two breaks. The part you are have referred to shows the diff where the break from 1220 has been removed and the break on 1215 has been added. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #58: Update runbroker.sh
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/58 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #124: ROCKETMQ-213 :
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/124 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #125: ROCKETMQ-205 :
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/125 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #71: add telnet CMD to nameServer
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/71 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/138 Hi, @Jaskey could you please have a look at this PR? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/120 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #126: [ROCKETMQ-231] fix pull consumer pull result ...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/126 [![Coverage Status](https://coveralls.io/builds/12636404/badge)](https://coveralls.io/builds/12636404) Coverage increased (+0.9%) to 39.575% when pulling **a5d73d080e49e63457c2be4b996c339d6e488fae on lindzh:fix_consumer_pull_msg_size** into **9bb6eae4bd35697808174a2ff9195e393450ec91 on apache:master**. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #141: [ROCKETMQ-254] fix logger appender unit test ...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/141 [![Coverage Status](https://coveralls.io/builds/12636199/badge)](https://coveralls.io/builds/12636199) Coverage decreased (-0.4%) to 38.718% when pulling **6cdfaacb2e01eae477d561c8f6efd0fed1afce09 on lindzh:logger_appender_test** into **d4149207e27ed3516f1f06407b55986790b806ae on apache:master**. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #126: [ROCKETMQ-231] fix pull consumer pull ...
Github user lindzh commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/126#discussion_r130558354 --- Diff: store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java --- @@ -1110,7 +1110,7 @@ private boolean isTheBatchFull(int sizePy, int maxMsgNums, int bufferTotal, int return false; } -if ((messageTotal + 1) >= maxMsgNums) { --- End diff -- Yes,this problem has been fixed to maxMsgNums <= messageTotal --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...
Github user vsair commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/120#discussion_r130556323 --- Diff: client/src/main/java/org/apache/rocketmq/client/log/ClientLogger.java --- @@ -99,7 +111,12 @@ private static Logger createLogger(final String loggerName) { } public static Logger getLog() { -return log; +if (log == null) { --- End diff -- why not modify the init method? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
Re: The new branching model has been put into effect.
Hi Roman, The last release didn't do a right merge between develop and master branch, so we saw a big commit gap. I have made a synchronization between these two branches just now. Regards On Tue, Aug 1, 2017 at 4:12 PM, Roman Shtykhwrote: > Hi Yukon, > Can you confirm the release was done correctly (with all modifications > incorporated, as advertised)? > -- Roman > > > On Friday, July 14, 2017 10:37 AM, yukon wrote: > > > Hi, > > > Thanks guys, actually the release manager will do a sync between `master` > and `develop`(duplex merge) when release a new version, so this diff only > will occur in the gap between two releases. > > But... Maybe there is something wrong, I will check it~ > > > Regards, > yukon > > On Thu, Jul 13, 2017 at 10:00 AM, Zhanhui Li wrote: > > > Agree, develop and master branches should not split. We need to fix this > > issue. > > > > > 在 2017年7月12日,下午9:23,Roman Shtykh 写道: > > > > > > Folks, > > > Please have a look at https://github.com/apache/ > incubator-rocketmq/tree/ > > develop > > > This branch is 82 commits ahead, 74 commits behind master. <-- how can > > this happen? considering we had a recent release. > > > Do we have the situation Justin described before -- "In project where > we > > had this split it just ends up being more work, people accidentally check > > into the wrong branch and I’ve see a lot of merge issues"? > > > > > > I'll say it again, but it is very inconvenient to use gitflow model on > > github (when on github you normally commit on master branch). > > > > > > -- Roman > > > > > >On Friday, March 17, 2017 11:41 AM, Justin Mclean < > > jus...@classsoftware.com> wrote: > > > > > > > > > Hi, > > > > > > I also don't see there's a real need to split master and develop. In > > project where we had this split it just ends up being more work, people > > accidentally check into the wrong branch and I’ve see a lot of merge > issues. > > > > > > That being said the project is free to select whatever model they want. > > > > > > Thanks, > > > Justin > > > > > > > > > >
[GitHub] incubator-rocketmq issue #120: [ROCKETMQ-224] add rocketmq client log4j2 log...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/120 [![Coverage Status](https://coveralls.io/builds/12635815/badge)](https://coveralls.io/builds/12635815) Coverage increased (+0.2%) to 38.771% when pulling **0ea6994712784d700aa365c2e121306b898b3355 on lindzh:fix_client_logger** into **0c5e53db6f4d0ed9f25747379a8b679e2da5392d on apache:master**. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #120: [ROCKETMQ-224] add rocketmq client log4j2 log...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/120 [![Coverage Status](https://coveralls.io/builds/12635815/badge)](https://coveralls.io/builds/12635815) Coverage increased (+0.2%) to 38.771% when pulling **0ea6994712784d700aa365c2e121306b898b3355 on lindzh:fix_client_logger** into **0c5e53db6f4d0ed9f25747379a8b679e2da5392d on apache:master**. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #120: [ROCKETMQ-224] add rocketmq client log4j2 log...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/120 [![Coverage Status](https://coveralls.io/builds/12635815/badge)](https://coveralls.io/builds/12635815) Coverage increased (+0.2%) to 38.771% when pulling **0ea6994712784d700aa365c2e121306b898b3355 on lindzh:fix_client_logger** into **0c5e53db6f4d0ed9f25747379a8b679e2da5392d on apache:master**. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #141: [ROCKETMQ-254] fix logger appender uni...
Github user lindzh commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/141#discussion_r130553480 --- Diff: logappender/src/main/java/org/apache/rocketmq/logappender/common/ProducerInstance.java --- @@ -61,7 +68,7 @@ public static MQProducer getInstance(String nameServerAddress, String group) thr defaultMQProducer.setNamesrvAddr(nameServerAddress); MQProducer beforeProducer = null; //cas put producer --- End diff -- This comment has been deleted. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #120: [ROCKETMQ-224] add rocketmq client log4j2 log...
Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/120 Now, LGTM. this pr also fix the rocketmq's logger appender bug when using any concrete implementation, no matter log4j, log4j2 or logback. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...
Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/120#discussion_r130550009 --- Diff: client/src/test/java/org/apache/rocketmq/client/log/ClientLogTest.java --- @@ -32,15 +33,19 @@ LOG_DIR = System.getProperty(CLIENT_LOG_ROOT, "${user.home}/logs/rocketmqlogs"); } +// FIXME: Workarond for concret implementation for slf4j, is there any better solution for all slf4j implementations in one class ? 2017/8/1 --- End diff -- we'd better remember this task and fix it later. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
Re: The new branching model has been put into effect.
Hi Yukon, Can you confirm the release was done correctly (with all modifications incorporated, as advertised)? -- Roman On Friday, July 14, 2017 10:37 AM, yukonwrote: Hi, Thanks guys, actually the release manager will do a sync between `master` and `develop`(duplex merge) when release a new version, so this diff only will occur in the gap between two releases. But... Maybe there is something wrong, I will check it~ Regards, yukon On Thu, Jul 13, 2017 at 10:00 AM, Zhanhui Li wrote: > Agree, develop and master branches should not split. We need to fix this > issue. > > > 在 2017年7月12日,下午9:23,Roman Shtykh 写道: > > > > Folks, > > Please have a look at https://github.com/apache/incubator-rocketmq/tree/ > develop > > This branch is 82 commits ahead, 74 commits behind master. <-- how can > this happen? considering we had a recent release. > > Do we have the situation Justin described before -- "In project where we > had this split it just ends up being more work, people accidentally check > into the wrong branch and I’ve see a lot of merge issues"? > > > > I'll say it again, but it is very inconvenient to use gitflow model on > github (when on github you normally commit on master branch). > > > > -- Roman > > > > On Friday, March 17, 2017 11:41 AM, Justin Mclean < > jus...@classsoftware.com> wrote: > > > > > > Hi, > > > > I also don't see there's a real need to split master and develop. In > project where we had this split it just ends up being more work, people > accidentally check into the wrong branch and I’ve see a lot of merge issues. > > > > That being said the project is free to select whatever model they want. > > > > Thanks, > > Justin > > > >
[GitHub] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...
Github user lindzh commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/120#discussion_r130544050 --- Diff: client/src/main/java/org/apache/rocketmq/client/log/ClientLogger.java --- @@ -27,16 +28,19 @@ public static final String CLIENT_LOG_ROOT = "rocketmq.client.logRoot"; public static final String CLIENT_LOG_MAXINDEX = "rocketmq.client.logFileMaxIndex"; public static final String CLIENT_LOG_LEVEL = "rocketmq.client.logLevel"; + private static Logger log; -static { -log = createLogger(LoggerName.CLIENT_LOGGER_NAME); +public static Class logClass = null; --- End diff -- Fixed it with reflect --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #137: Develop
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/137 [![Coverage Status](https://coveralls.io/builds/12635031/badge)](https://coveralls.io/builds/12635031) Coverage decreased (-0.03%) to 39.09% when pulling **9bb6eae4bd35697808174a2ff9195e393450ec91 on develop** into **d4149207e27ed3516f1f06407b55986790b806ae on master**. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #137: Develop
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/137 [![Coverage Status](https://coveralls.io/builds/12635031/badge)](https://coveralls.io/builds/12635031) Coverage decreased (-0.03%) to 39.09% when pulling **9bb6eae4bd35697808174a2ff9195e393450ec91 on develop** into **d4149207e27ed3516f1f06407b55986790b806ae on master**. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #137: Develop
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/137 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #127: [ROCKETMQ-233] Fix pull interval issue
Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/127 @lizhanhui Got it, thanks! How about adding comments with explanations for each policy class? I think developers will appreciate that. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #127: [ROCKETMQ-233] Fix pull interval issue
Github user shroman commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/127#discussion_r130530039 --- Diff: client/src/main/java/org/apache/rocketmq/client/consumer/PullIntervalPolicy.java --- @@ -0,0 +1,25 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.rocketmq.client.consumer; + +public interface PullIntervalPolicy { + +void update(PullStatus status); + +int getPullInterval(); --- End diff -- What about making it `long`? _delay_ of executePullRequestLater(...) is long. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #139: [ROCKETMQ-242] mqclient can not fetch ...
Github user shroman commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/139#discussion_r130528221 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/factory/MQClientInstance.java --- @@ -255,19 +255,17 @@ public void start() throws MQClientException { } private void startScheduledTask() { -if (null == this.clientConfig.getNamesrvAddr()) { --- End diff -- I think what @qqeasonchen says makes sense, but we have discuss what behavior we want to have. To me, if addresses have been manually specified, it makes sense to fetch name servers only if a call to manually specified servers fail. No need to do periodic calls. What do you think? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #139: [ROCKETMQ-242] mqclient can not fetch ...
Github user qqeasonchen commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/139#discussion_r130527608 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/factory/MQClientInstance.java --- @@ -255,19 +255,17 @@ public void start() throws MQClientException { } private void startScheduledTask() { -if (null == this.clientConfig.getNamesrvAddr()) { --- End diff -- in fact, even though we specified the web endpoint, client will not fetch name server addresses any more beacase of below code in start(): `// If not specified,looking address from name server if (null == this.clientConfig.getNamesrvAddr()) { this.mQClientAPIImpl.fetchNameServerAddr(); }` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #134: Rito - #44 - Extracting out duplicated code f...
Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/134 @Ritabrata-TW In other words, your pull request description has to be in this format ``` [ROCKETMQ-xxx] Message describing the issue. Link to ROCKETMQ-xxx, which is the JIRA issue. ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #134: Rito - #44 - Extracting out duplicated...
Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130526510 --- Diff: store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java --- @@ -745,17 +751,7 @@ public long getMessageStoreTimeStamp(String topic, int queueId, long consumeQueu ConsumeQueue logicQueue = this.findConsumeQueue(topic, queueId); if (logicQueue != null) { SelectMappedBufferResult result = logicQueue.getIndexBuffer(consumeQueueOffset); -if (result != null) { -try { -final long phyOffset = result.getByteBuffer().getLong(); -final int size = result.getByteBuffer().getInt(); -long storeTime = this.getCommitLog().pickupStoreTimestamp(phyOffset, size); -return storeTime; -} catch (Exception ignored) { -} finally { -result.release(); -} -} +long storeTime = getStoreTime(result); --- End diff -- We appreciate your contribution for rocketmq commuity, could you use unit test to verify your refactoring --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #134: Rito - #44 - Extracting out duplicated code f...
Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/134 @Ritabrata-TW I am not understand your refer "Story number 44 from the Kanban Board", could you detail 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #141: [ROCKETMQ-254] fix logger appender uni...
Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/141#discussion_r130525133 --- Diff: logappender/src/main/java/org/apache/rocketmq/logappender/common/ProducerInstance.java --- @@ -61,7 +68,7 @@ public static MQProducer getInstance(String nameServerAddress, String group) thr defaultMQProducer.setNamesrvAddr(nameServerAddress); MQProducer beforeProducer = null; //cas put producer --- End diff -- what's the purpose for this comment :-) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---