Uploaded image for project: 'Apache RocketMQ'
  1. Apache RocketMQ
  2. ROCKETMQ-18

Repackage com.alibaba to org.apache and change maven coordinate

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 4.0.0-incubating
    • Fix Version/s: 4.0.0-incubating
    • Component/s: None
    • Labels:
    • Flags:
      Important

      Issue Links

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

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

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/21
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vongosling commented on the issue:

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

        That's ok

        Show
        githubbot ASF GitHub Bot added a comment - Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/21 That's ok
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vongosling commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103642

        — Diff: broker/src/main/java/org/apache/rocketmq/broker/filtersrv/FilterServerUtil.java —
        @@ -26,9 +26,9 @@ public static void callShell(final String shellString, final Logger log) {
        String[] cmdArray = splitShellString(shellString);
        process = Runtime.getRuntime().exec(cmdArray);
        process.waitFor();

        • log.info("callShell: <{}> OK", shellString);
          + log.info("CallShell: <{}> OK", shellString);
          } catch (Throwable e) {
        • log.error("callShell: readLine IOException, " + shellString, e);
          + log.error("CallShell: readLine IOException, {}", shellString, e);
            • End diff –

        Wrong usage for log.error, please review it

        Show
        githubbot ASF GitHub Bot added a comment - Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103642 — Diff: broker/src/main/java/org/apache/rocketmq/broker/filtersrv/FilterServerUtil.java — @@ -26,9 +26,9 @@ public static void callShell(final String shellString, final Logger log) { String[] cmdArray = splitShellString(shellString); process = Runtime.getRuntime().exec(cmdArray); process.waitFor(); log.info("callShell: <{}> OK", shellString); + log.info("CallShell: <{}> OK", shellString); } catch (Throwable e) { log.error("callShell: readLine IOException, " + shellString, e); + log.error("CallShell: readLine IOException, {}", shellString, e); End diff – Wrong usage for log.error, please review it
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vongosling commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103499

        — Diff: broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java —
        @@ -497,7 +497,7 @@ private void printMasterAndSlaveDiff() {
        long diff = this.messageStore.slaveFallBehindMuch();

        // XXX: warn and notify me

        • log.info("slave fall behind master, how much, {} bytes", diff);
          + log.info("Slave fall behind master, how much, {} bytes", diff);
            • End diff –

        what's how much meaning?

        Show
        githubbot ASF GitHub Bot added a comment - Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103499 — Diff: broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java — @@ -497,7 +497,7 @@ private void printMasterAndSlaveDiff() { long diff = this.messageStore.slaveFallBehindMuch(); // XXX: warn and notify me log.info("slave fall behind master, how much, {} bytes", diff); + log.info("Slave fall behind master, how much, {} bytes", diff); End diff – what's how much meaning?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vongosling commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103857

        — Diff: broker/src/main/java/org/apache/rocketmq/broker/slave/SlaveSynchronize.java —
        @@ -106,12 +106,12 @@ private void syncDelayOffset() {
        try

        { MixAll.string2File(delayOffset, fileName); }

        catch (IOException e) {

        • log.error("persist file Exception, " + fileName, e);
          + log.error("Persist file Exception, {}", fileName, e);
          }
          }
        • log.info("update slave delay offset from master, {}", masterAddrBak);
          + log.info("Update slave delay offset from master, {}", masterAddrBak);
          } catch (Exception e) {
        • log.error("syncDelayOffset Exception, " + masterAddrBak, e);
          + log.error("SyncDelayOffset Exception, {}", masterAddrBak, e);
            • End diff –

        wrong

        Show
        githubbot ASF GitHub Bot added a comment - Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103857 — Diff: broker/src/main/java/org/apache/rocketmq/broker/slave/SlaveSynchronize.java — @@ -106,12 +106,12 @@ private void syncDelayOffset() { try { MixAll.string2File(delayOffset, fileName); } catch (IOException e) { log.error("persist file Exception, " + fileName, e); + log.error("Persist file Exception, {}", fileName, e); } } log.info("update slave delay offset from master, {}", masterAddrBak); + log.info("Update slave delay offset from master, {}", masterAddrBak); } catch (Exception e) { log.error("syncDelayOffset Exception, " + masterAddrBak, e); + log.error("SyncDelayOffset Exception, {}", masterAddrBak, e); End diff – wrong
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vongosling commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103841

        — Diff: broker/src/main/java/org/apache/rocketmq/broker/slave/SlaveSynchronize.java —
        @@ -85,9 +85,9 @@ private void syncConsumerOffset() {
        this.brokerController.getConsumerOffsetManager().getOffsetTable()
        .putAll(offsetWrapper.getOffsetTable());
        this.brokerController.getConsumerOffsetManager().persist();

        • log.info("update slave consumer offset from master, {}", masterAddrBak);
          + log.info("Update slave consumer offset from master, {}", masterAddrBak);
          } catch (Exception e) {
        • log.error("syncConsumerOffset Exception, " + masterAddrBak, e);
          + log.error("SyncConsumerOffset Exception, {}", masterAddrBak, e);
            • End diff –

        Wrong

        Show
        githubbot ASF GitHub Bot added a comment - Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103841 — Diff: broker/src/main/java/org/apache/rocketmq/broker/slave/SlaveSynchronize.java — @@ -85,9 +85,9 @@ private void syncConsumerOffset() { this.brokerController.getConsumerOffsetManager().getOffsetTable() .putAll(offsetWrapper.getOffsetTable()); this.brokerController.getConsumerOffsetManager().persist(); log.info("update slave consumer offset from master, {}", masterAddrBak); + log.info("Update slave consumer offset from master, {}", masterAddrBak); } catch (Exception e) { log.error("syncConsumerOffset Exception, " + masterAddrBak, e); + log.error("SyncConsumerOffset Exception, {}", masterAddrBak, e); End diff – Wrong
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vongosling commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103845

        — Diff: broker/src/main/java/org/apache/rocketmq/broker/slave/SlaveSynchronize.java —
        @@ -106,12 +106,12 @@ private void syncDelayOffset() {
        try

        { MixAll.string2File(delayOffset, fileName); }

        catch (IOException e) {

        • log.error("persist file Exception, " + fileName, e);
          + log.error("Persist file Exception, {}", fileName, e);
            • End diff –

        wrong

        Show
        githubbot ASF GitHub Bot added a comment - Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103845 — Diff: broker/src/main/java/org/apache/rocketmq/broker/slave/SlaveSynchronize.java — @@ -106,12 +106,12 @@ private void syncDelayOffset() { try { MixAll.string2File(delayOffset, fileName); } catch (IOException e) { log.error("persist file Exception, " + fileName, e); + log.error("Persist file Exception, {}", fileName, e); End diff – wrong
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vongosling commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103915

        — Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPullConsumerImpl.java —
        @@ -228,6 +228,7 @@ public void subscriptionAutomatically(final String topic)

        { topic, SubscriptionData.SUB_ALL); this.rebalanceImpl.subscriptionInner.putIfAbsent(topic, subscriptionData); }

        catch (Exception e) {
        + log.error("", e);
        — End diff –

        So what would you do if you had all the security in the world?

        Show
        githubbot ASF GitHub Bot added a comment - Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103915 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPullConsumerImpl.java — @@ -228,6 +228,7 @@ public void subscriptionAutomatically(final String topic) { topic, SubscriptionData.SUB_ALL); this.rebalanceImpl.subscriptionInner.putIfAbsent(topic, subscriptionData); } catch (Exception e) { + log.error("", e); — End diff – So what would you do if you had all the security in the world?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vongosling commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103929

        — Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPullConsumerImpl.java —
        @@ -247,6 +248,7 @@ public void executeHookBefore(final ConsumeMessageContext context) {
        try

        { hook.consumeMessageBefore(context); }

        catch (Throwable e) {
        + log.error("", e);
        — End diff –

        So what would you do if you had all the security in the world?

        Show
        githubbot ASF GitHub Bot added a comment - Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103929 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPullConsumerImpl.java — @@ -247,6 +248,7 @@ public void executeHookBefore(final ConsumeMessageContext context) { try { hook.consumeMessageBefore(context); } catch (Throwable e) { + log.error("", e); — End diff – So what would you do if you had all the security in the world?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vongosling commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103833

        — Diff: broker/src/main/java/org/apache/rocketmq/broker/slave/SlaveSynchronize.java —
        @@ -68,10 +68,10 @@ private void syncTopicConfig() {
        .putAll(topicWrapper.getTopicConfigTable());
        this.brokerController.getTopicConfigManager().persist();

        • log.info("update slave topic config from master, {}", masterAddrBak);
          + log.info("Update slave topic config from master, {}", masterAddrBak);
          }
          } catch (Exception e) {
        • log.error("syncTopicConfig Exception, " + masterAddrBak, e);
          + log.error("SyncTopicConfig Exception, {}", masterAddrBak, e);
            • End diff –

        Wrong usage

        Show
        githubbot ASF GitHub Bot added a comment - Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103833 — Diff: broker/src/main/java/org/apache/rocketmq/broker/slave/SlaveSynchronize.java — @@ -68,10 +68,10 @@ private void syncTopicConfig() { .putAll(topicWrapper.getTopicConfigTable()); this.brokerController.getTopicConfigManager().persist(); log.info("update slave topic config from master, {}", masterAddrBak); + log.info("Update slave topic config from master, {}", masterAddrBak); } } catch (Exception e) { log.error("syncTopicConfig Exception, " + masterAddrBak, e); + log.error("SyncTopicConfig Exception, {}", masterAddrBak, e); End diff – Wrong usage
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vongosling commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103864

        — Diff: broker/src/main/java/org/apache/rocketmq/broker/slave/SlaveSynchronize.java —
        @@ -134,10 +134,10 @@ private void syncSubscriptionGroupConfig() {
        subscriptionGroupManager.getSubscriptionGroupTable().putAll(
        subscriptionWrapper.getSubscriptionGroupTable());
        subscriptionGroupManager.persist();

        • log.info("update slave Subscription Group from master, {}", masterAddrBak);
          + log.info("Update slave Subscription Group from master, {}", masterAddrBak);
          }
          } catch (Exception e) {
        • log.error("syncSubscriptionGroup Exception, " + masterAddrBak, e);
          + log.error("SyncSubscriptionGroup Exception, {}", masterAddrBak, e);
            • End diff –

        wrong

        Show
        githubbot ASF GitHub Bot added a comment - Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103864 — Diff: broker/src/main/java/org/apache/rocketmq/broker/slave/SlaveSynchronize.java — @@ -134,10 +134,10 @@ private void syncSubscriptionGroupConfig() { subscriptionGroupManager.getSubscriptionGroupTable().putAll( subscriptionWrapper.getSubscriptionGroupTable()); subscriptionGroupManager.persist(); log.info("update slave Subscription Group from master, {}", masterAddrBak); + log.info("Update slave Subscription Group from master, {}", masterAddrBak); } } catch (Exception e) { log.error("syncSubscriptionGroup Exception, " + masterAddrBak, e); + log.error("SyncSubscriptionGroup Exception, {}", masterAddrBak, e); End diff – wrong
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vongosling commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103933

        — Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPullConsumerImpl.java —
        @@ -258,6 +260,7 @@ public void executeHookAfter(final ConsumeMessageContext context) {
        try

        { hook.consumeMessageAfter(context); }

        catch (Throwable e) {
        + log.error("", e);
        — End diff –

        So what would you do if you had all the security in the world?

        Show
        githubbot ASF GitHub Bot added a comment - Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94103933 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPullConsumerImpl.java — @@ -258,6 +260,7 @@ public void executeHookAfter(final ConsumeMessageContext context) { try { hook.consumeMessageAfter(context); } catch (Throwable e) { + log.error("", e); — End diff – So what would you do if you had all the security in the world?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user zhouxinyu commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94101719

        — Diff: store/src/main/java/org/apache/rocketmq/store/CommitLog.java —
        @@ -1150,15 +1150,15 @@ public AppendMessageResult doAppend(final long fileFromOffset, final ByteBuffer
        final byte[] propertiesData =
        msgInner.getPropertiesString() == null ? null : msgInner.getPropertiesString().getBytes(MessageDecoder.CHARSET_UTF8);

        • final short propertiesLength = propertiesData == null ? 0 : (short) propertiesData.length;
          + final int propertiesLength = propertiesData == null ? 0 : propertiesData.length;

        if (propertiesLength > Short.MAX_VALUE) {
        log.warn("putMessage message properties length too long. length={}", propertiesData.length);
        return new AppendMessageResult(AppendMessageStatus.PROPERTIES_SIZE_EXCEEDED);
        }

        final byte[] topicData = msgInner.getTopic().getBytes(MessageDecoder.CHARSET_UTF8);

        • final int topicLength = topicData == null ? 0 : topicData.length;
            • End diff –

        topicData may be null.

        Show
        githubbot ASF GitHub Bot added a comment - Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94101719 — Diff: store/src/main/java/org/apache/rocketmq/store/CommitLog.java — @@ -1150,15 +1150,15 @@ public AppendMessageResult doAppend(final long fileFromOffset, final ByteBuffer final byte[] propertiesData = msgInner.getPropertiesString() == null ? null : msgInner.getPropertiesString().getBytes(MessageDecoder.CHARSET_UTF8); final short propertiesLength = propertiesData == null ? 0 : (short) propertiesData.length; + final int propertiesLength = propertiesData == null ? 0 : propertiesData.length; if (propertiesLength > Short.MAX_VALUE) { log.warn("putMessage message properties length too long. length={}", propertiesData.length); return new AppendMessageResult(AppendMessageStatus.PROPERTIES_SIZE_EXCEEDED); } final byte[] topicData = msgInner.getTopic().getBytes(MessageDecoder.CHARSET_UTF8); final int topicLength = topicData == null ? 0 : topicData.length; End diff – topicData may be null.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user zhouxinyu commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94101409

        — Diff: client/src/main/java/org/apache/rocketmq/client/impl/factory/MQClientInstance.java —
        @@ -1134,10 +1131,10 @@ public ConsumerRunningInfo consumerRunningInfo(final String consumerGroup) {

        List<String> nsList = this.mQClientAPIImpl.getRemotingClient().getNameServerAddressList();

        • StringBuffer strBuffer = new StringBuffer();
          + StringBuilder strBuffer = new StringBuilder();
            • End diff –

        use strBuilder

        Show
        githubbot ASF GitHub Bot added a comment - Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94101409 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/factory/MQClientInstance.java — @@ -1134,10 +1131,10 @@ public ConsumerRunningInfo consumerRunningInfo(final String consumerGroup) { List<String> nsList = this.mQClientAPIImpl.getRemotingClient().getNameServerAddressList(); StringBuffer strBuffer = new StringBuffer(); + StringBuilder strBuffer = new StringBuilder(); End diff – use strBuilder
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user zhouxinyu commented on a diff in the pull request:

        https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94101345

        — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java —
        @@ -469,6 +467,7 @@ private void onExceptionImpl(final String brokerName, //
        try

        { sendCallback.onException(e); }

        catch (Exception e2) {
        + log.error("", e2);
        — End diff –

        // Ignore

        Show
        githubbot ASF GitHub Bot added a comment - Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94101345 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java — @@ -469,6 +467,7 @@ private void onExceptionImpl(final String brokerName, // try { sendCallback.onException(e); } catch (Exception e2) { + log.error("", e2); — End diff – // Ignore
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user lollipopjin commented on the issue:

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

        @lizhanhui
        `
        <!-- compiler settings properties -->

        <maven.compiler.source>1.7</maven.compiler.source>

        <maven.compiler.target>1.7</maven.compiler.target>
        ` Already

        Show
        githubbot ASF GitHub Bot added a comment - Github user lollipopjin commented on the issue: https://github.com/apache/incubator-rocketmq/pull/21 @lizhanhui ` <!-- compiler settings properties --> <maven.compiler.source>1.7</maven.compiler.source> <maven.compiler.target>1.7</maven.compiler.target> ` Already
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user lizhanhui commented on the issue:

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

        @zhouxinyu This PR introduces grammar of Java 7, say diamond. Do you plan to upgrade the language compatible version?

        Show
        githubbot ASF GitHub Bot added a comment - Github user lizhanhui commented on the issue: https://github.com/apache/incubator-rocketmq/pull/21 @zhouxinyu This PR introduces grammar of Java 7, say diamond. Do you plan to upgrade the language compatible version?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user zhouxinyu commented on the issue:

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

        It seems ok for me. @lollipopjin please review this pr.

        Show
        githubbot ASF GitHub Bot added a comment - Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/21 It seems ok for me. @lollipopjin please review this pr.
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user dongeforever opened a pull request:

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

        ROCKETMQ-18Clean code prompted by IntelliJ

        Clean code prompted by IntelliJ for branch 'ROCKETMQ-18'.

        @vongosling @zhouxinyu please take a time to review it.

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

        $ git pull https://github.com/dongeforever/incubator-rocketmq ROCKETMQ-18

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

        https://github.com/apache/incubator-rocketmq/pull/21.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 #21


        commit e644425ae059dbf4bfe9ad2d134a98f94d32ded1
        Author: dongeforever <zhendongliu92@yeah.net>
        Date: 2016-12-28T11:11:21Z

        Clean code prompted by IntelliJ


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user dongeforever opened a pull request: https://github.com/apache/incubator-rocketmq/pull/21 ROCKETMQ-18 Clean code prompted by IntelliJ Clean code prompted by IntelliJ for branch ' ROCKETMQ-18 '. @vongosling @zhouxinyu please take a time to review it. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dongeforever/incubator-rocketmq ROCKETMQ-18 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-rocketmq/pull/21.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 #21 commit e644425ae059dbf4bfe9ad2d134a98f94d32ded1 Author: dongeforever <zhendongliu92@yeah.net> Date: 2016-12-28T11:11:21Z Clean code prompted by IntelliJ

          People

          • Assignee:
            Yukon yukon
            Reporter:
            vongosling vongosling
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Due:
              Created:
              Updated:
              Resolved:

              Development