You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by shroman <gi...@git.apache.org> on 2017/02/16 09:30:59 UTC
[GitHub] incubator-rocketmq pull request #65: [ROCKETMQ-104] Make MQAdmin commands th...
GitHub user shroman opened a pull request:
https://github.com/apache/incubator-rocketmq/pull/65
[ROCKETMQ-104] Make MQAdmin commands throw exceptions
JIRA issue: https://issues.apache.org/jira/browse/ROCKETMQ-104
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shroman/incubator-rocketmq ROCKETMQ-104
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-rocketmq/pull/65.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 #65
----
commit 426b3f37686e6df05563d654493efc72e374ec40
Author: shtykh_roman <rs...@yahoo.com>
Date: 2017-02-16T09:28:20Z
[ROCKETMQ-104] Make MQAdmin commands throw exceptions
JIRA issue: https://issues.apache.org/jira/browse/ROCKETMQ-104
----
---
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 #65: [ROCKETMQ-104] Make MQAdmin commands throw exc...
Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/65
@shroman Could you help to close this PR and 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #65: [ROCKETMQ-104] Make MQAdmin commands throw exc...
Posted by zhouxinyu <gi...@git.apache.org>.
Github user zhouxinyu commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/65
please @vongosling @lizhanhui help review.
---
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 #65: [ROCKETMQ-104] Make MQAdmin commands throw exc...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/65
@zhouxinyu Thanks for checking it!
Well, `@Ignore` is there just for finer granularity -- we remove it one by one when implementing each test. If you think we don't need it, I can remove ;)
---
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 #65: [ROCKETMQ-104] Make MQAdmin commands th...
Posted by shroman <gi...@git.apache.org>.
Github user shroman closed the pull request at:
https://github.com/apache/incubator-rocketmq/pull/65
---
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 #65: [ROCKETMQ-104] Make MQAdmin commands th...
Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/65#discussion_r103126453
--- Diff: tools/src/main/java/org/apache/rocketmq/tools/command/SubCommand.java ---
@@ -21,11 +21,11 @@
import org.apache.rocketmq.remoting.RPCHook;
public interface SubCommand {
- public String commandName();
+ String commandName();
--- End diff --
Cool. JLS points out that the interface method is public abstract
---
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 #65: [ROCKETMQ-104] Make MQAdmin commands th...
Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/65#discussion_r103126511
--- Diff: tools/src/main/java/org/apache/rocketmq/tools/command/offset/ResetOffsetByTimeCommand.java ---
@@ -22,25 +22,16 @@
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.Option;
import org.apache.commons.cli.Options;
-import org.apache.commons.cli.PosixParser;
import org.apache.rocketmq.client.exception.MQClientException;
import org.apache.rocketmq.common.UtilAll;
import org.apache.rocketmq.common.message.MessageQueue;
import org.apache.rocketmq.common.protocol.ResponseCode;
import org.apache.rocketmq.remoting.RPCHook;
-import org.apache.rocketmq.srvutil.ServerUtil;
import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
import org.apache.rocketmq.tools.command.SubCommand;
+import org.apache.rocketmq.tools.command.SubCommandException;
public class ResetOffsetByTimeCommand implements SubCommand {
- public static void main(String[] args) {
--- End diff --
+1
---
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 #65: [ROCKETMQ-104] Make MQAdmin commands throw exc...
Posted by zhouxinyu <gi...@git.apache.org>.
Github user zhouxinyu commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/65
Hi @shroman ,
LGTM, but why do we add so many `@Ignore` annotations in this PR? All the he unit tests of tools module have been skipped in `tools/pom.xml`. We may forget to remove these annotations after this issue[1] has been resolved.
[1]. https://issues.apache.org/jira/browse/ROCKETMQ-77
---
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 #65: [ROCKETMQ-104] Make MQAdmin commands throw exc...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/65
[](https://coveralls.io/builds/10192313)
Coverage increased (+0.02%) to 31.538% when pulling **426b3f37686e6df05563d654493efc72e374ec40 on shroman:ROCKETMQ-104** into **573b22c37806a21543b90707bcce6022243a62da 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #65: [ROCKETMQ-104] Make MQAdmin commands throw exc...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/65
@vongosling Thanks for reviewing!
Sure, unit tests can be completed after we improve this module.
---
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 #65: [ROCKETMQ-104] Make MQAdmin commands throw exc...
Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/65
@shroman IMO, we can let the tool module unit-test passed once someone polish here. thoughts?
---
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 #65: [ROCKETMQ-104] Make MQAdmin commands throw exc...
Posted by zhouxinyu <gi...@git.apache.org>.
Github user zhouxinyu commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/65
Hi, @shroman this PR will be merged to `develop` branch firstly as our new branching model: http://rocketmq.incubator.apache.org/docs/branching-model
---
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.
---