[GitHub] incubator-rocketmq pull request #38: [ROCKETMQ-44] Refactor to avoid duplica...
Github user wu-sheng commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/38#discussion_r97021619
--- Diff:
store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java ---
@@ -1278,7 +1282,8 @@ public void doDispatch(DispatchRequest req) {
}
}
-public void putMessagePositionInfo(String topic, int queueId, long
offset, int size, long tagsCode, long storeTimestamp,
+public void putMessagePositionInfo(String topic, int queueId, long
offset, int size, long tagsCode,
+long storeTimestamp,
--- End diff --
@vongosling , I am not sure what in checkstyle trigger this. And what is
**UT**? Sorry, no clue. ð
---
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 #35: [ROCKETMQ-42] Output is not friendly, when Mai...
Github user wu-sheng commented on the issue: https://github.com/apache/incubator-rocketmq/pull/35 Whether the pull request be merged or not, the project team should close it without anything. Using a commit to close, only comment on "won't fix", seem not regular for outside contributors. More communications should stay on GitHub, rather than your inside team, as least have a discussion, and then have a conclusion. In **Apache Software Foundation**, `benevolent dictator` is not welcome, I think you should pay attention to this. This is only a suggestion, but I think this is important. btw @vongosling @WillemJiang --- 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 #45: [ROCKETMQ-54] Polish unit tests for rocketmq-n...
Github user iskl commented on the issue: https://github.com/apache/incubator-rocketmq/pull/45 I've merged this branch from remote master. The integration test on OSX are all passed. Thanks~ --- 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 #38: [ROCKETMQ-44] Refactor to avoid duplicated cod...
Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/38 @wu-sheng Thanks for your clean code actions. Why so many format operation, what had happend when you use our checkstyle, let me know. BTW, you know, UT is the best assistant of the 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 #45: [ROCKETMQ-54] Polish unit tests for rocketmq-n...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/45 [](https://coveralls.io/builds/9760902) Coverage increased (+12.3%) to 25.826% when pulling **f078eed8248b16c229ece1a7b948eb5ccdc3d1b0 on iskl:ROCKETMQ-54** into **1c529533e12882c112f3a3f3a3500c65c531da50 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 #38: [ROCKETMQ-44] Refactor to avoid duplicated cod...
Github user wu-sheng commented on the issue: https://github.com/apache/incubator-rocketmq/pull/38 @zhouxinyu , If your team member can provide these Mock-Classes, it will be easier to add test-cases for my changes. --- 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 #38: [ROCKETMQ-44] Refactor to avoid duplicated cod...
Github user wu-sheng commented on the issue: https://github.com/apache/incubator-rocketmq/pull/38 @zhouxinyu , I have tried before. But there is a problem. In recent RocketMQ codes, there are no `ConsumeQueueMock` and `CommitLogMock`, I am not sure if I create the Mock-Classes, they are good enough. If you want me to do that, I can try it, but no guarantee. Any thoughts? @vongosling @WillemJiang --- 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 #46: [RocketMQ-58] Add integration test for ...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/46 --- 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 #46: [RocketMQ-58] Add integration test for ...
Github user zhouxinyu commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/46#discussion_r96842822
--- Diff:
test/src/test/java/org/apache/rocketmq/test/client/producer/querymsg/QueryMsgByTopicExceptionIT.java
---
@@ -0,0 +1,24 @@
+/*
+ * 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.test.client.producer.querymsg;
+
+public class QueryMsgByTopicExceptionIT {
+//@Test
--- End diff --
Empty class~
---
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 #44: [ROCKETMQ-64] Remove duplication code l...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/44 --- 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 #43: [ROCKETMQ-59] Change Charset usages in ...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/43 --- 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 #38: [ROCKETMQ-44] Refactor to avoid duplicated cod...
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/38 Hi @wu-sheng ,Could you please add some unit tests for 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 issue #44: [ROCKETMQ-64] Remove duplication code line in ...
Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/44 Thanks @naughtybear --- 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. ---
