You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/08/01 11:53:22 UTC

[GitHub] [rocketmq] aaron-ai opened a new pull request, #4756: Some improvement about branch management

aaron-ai opened a new pull request, #4756:
URL: https://github.com/apache/rocketmq/pull/4756

   Fixes #4755 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] lizhanhui commented on pull request #4756: Some improvement about branch management

Posted by GitBox <gi...@apache.org>.

lizhanhui commented on PR #4756:
URL: https://github.com/apache/rocketmq/pull/4756#issuecomment-1201105747

   Looks good. 
   +1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] coveralls commented on pull request #4756: [ISSUE #4755] Some improvement about branch management

Posted by GitBox <gi...@apache.org>.

coveralls commented on PR #4756:
URL: https://github.com/apache/rocketmq/pull/4756#issuecomment-1201207157

   
   [![Coverage Status](https://coveralls.io/builds/51310504/badge)](https://coveralls.io/builds/51310504)
   
   Coverage increased (+0.04%) to 48.899% when pulling **8248539a517bff6e713357cc80b7515412711edf on aaron-ai:pr** into **6a95661291a7a94e55b6813d5298b58a779aea2b on apache:develop**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] lizhanhui commented on pull request #4756: [ISSUE #4755] Some improvement about branch management

Posted by GitBox <gi...@apache.org>.

lizhanhui commented on PR #4756:
URL: https://github.com/apache/rocketmq/pull/4756#issuecomment-1229675485

@vintagewang

Actually, 2 is practically the number Google takes internally. Here is reason they give

> In practice, most code reviews that require more than one approval usually go through a two-step process: gaining an LGTM from a peer engineer, and then seeking approval from appropriate code owner/readability reviewer(s). This allows the two roles to focus on different aspects of the code review and saves review time. The primary reviewer can focus on code correctness and the general validity of the code change; the code owner can focus on whether this change is appropriate for their part of the codebase without having to focus on the details of each line of code. An approver is often looking for something different than a peer reviewer, in other words. After all, someone is trying to check in code to their project/directory. They are more concerned with questions such as: “Will this code be easy or difficult to maintain?” “Does it add to my technical debt?” “Do we have the expertise to maintain it within our team?”

> If all three of these types of reviews can be handled by one reviewer, why not just have those types of reviewers handle all code reviews? The short answer is scale. Separating the three roles adds flexibility to the code review process. If you are working with a peer on a new function within a utility library, you can get someone on your team to review the code for code correctness and comprehension. After several rounds (perhaps over several days), your code satisfies your peer reviewer and you get an LGTM. Now, you need only get an owner of the library (and owners often have appropriate readability) to approve the change.

See https://abseil.io/resources/swe-book/html/ch09.html#code_review-id00002

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] duhenglucky merged pull request #4756: [ISSUE #4755] Some improvement about branch management

Posted by GitBox <gi...@apache.org>.

duhenglucky merged PR #4756:
URL: https://github.com/apache/rocketmq/pull/4756


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] vintagewang commented on pull request #4756: [ISSUE #4755] Some improvement about branch management

Posted by GitBox <gi...@apache.org>.

vintagewang commented on PR #4756:
URL: https://github.com/apache/rocketmq/pull/4756#issuecomment-1229347877

   I was wondering if this "required_approving_review_count" would be more appropriate to set up to 2 and wanted to hear from other people.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] aaron-ai commented on pull request #4756: [ISSUE #4755] Some improvement about branch management

Posted by GitBox <gi...@apache.org>.

aaron-ai commented on PR #4756:
URL: https://github.com/apache/rocketmq/pull/4756#issuecomment-1202075056

   > LGTM~, Maybe more labels can be attached to RocketMQ, Messaging, Streaming, Eventing, CloudNative, etc.
   
   done.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] codecov-commenter commented on pull request #4756: [ISSUE #4755] Some improvement about branch management

Posted by GitBox <gi...@apache.org>.

codecov-commenter commented on PR #4756:
URL: https://github.com/apache/rocketmq/pull/4756#issuecomment-1201207545

   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4756?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#4756](https://codecov.io/gh/apache/rocketmq/pull/4756?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8248539) into [develop](https://codecov.io/gh/apache/rocketmq/commit/6a95661291a7a94e55b6813d5298b58a779aea2b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6a95661) will **increase** coverage by `0.25%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #4756      +/-   ##
   =============================================
   + Coverage      44.60%   44.85%   +0.25%     
   - Complexity      7544     7582      +38     
   =============================================
     Files            977      977              
     Lines          67777    67777              
     Branches        8959     8959              
   =============================================
   + Hits           30230    30403     +173     
   + Misses         33798    33627     -171     
   + Partials        3749     3747       -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/4756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...etmq/namesrv/routeinfo/BatchUnRegisterService.java](https://codecov.io/gh/apache/rocketmq/pull/4756/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bmFtZXNydi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbmFtZXNydi9yb3V0ZWluZm8vQmF0Y2hVblJlZ2lzdGVyU2VydmljZS5qYXZh) | `94.73% <0.00%> (-5.27%)` | :arrow_down: |
   | [...ava/org/apache/rocketmq/filter/util/BitsArray.java](https://codecov.io/gh/apache/rocketmq/pull/4756/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmlsdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9maWx0ZXIvdXRpbC9CaXRzQXJyYXkuamF2YQ==) | `59.82% <0.00%> (ø)` | |
   | [...e/rocketmq/namesrv/routeinfo/RouteInfoManager.java](https://codecov.io/gh/apache/rocketmq/pull/4756/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bmFtZXNydi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbmFtZXNydi9yb3V0ZWluZm8vUm91dGVJbmZvTWFuYWdlci5qYXZh) | `65.94% <0.00%> (+0.15%)` | :arrow_up: |
   | [...mq/client/impl/producer/DefaultMQProducerImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4756/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9wcm9kdWNlci9EZWZhdWx0TVFQcm9kdWNlckltcGwuamF2YQ==) | `44.74% <0.00%> (+0.37%)` | :arrow_up: |
   | [...cketmq/store/ha/autoswitch/AutoSwitchHAClient.java](https://codecov.io/gh/apache/rocketmq/pull/4756/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL2F1dG9zd2l0Y2gvQXV0b1N3aXRjaEhBQ2xpZW50LmphdmE=) | `75.84% <0.00%> (+0.37%)` | :arrow_up: |
   | [...rocketmq/broker/processor/PopMessageProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/4756/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL1BvcE1lc3NhZ2VQcm9jZXNzb3IuamF2YQ==) | `38.73% <0.00%> (+0.54%)` | :arrow_up: |
   | [.../org/apache/rocketmq/proxy/config/ProxyConfig.java](https://codecov.io/gh/apache/rocketmq/pull/4756/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHJveHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Byb3h5L2NvbmZpZy9Qcm94eUNvbmZpZy5qYXZh) | `49.45% <0.00%> (+0.54%)` | :arrow_up: |
   | [...ent/impl/consumer/DefaultLitePullConsumerImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4756/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9EZWZhdWx0TGl0ZVB1bGxDb25zdW1lckltcGwuamF2YQ==) | `70.48% <0.00%> (+1.01%)` | :arrow_up: |
   | [.../apache/rocketmq/logging/inner/LoggingBuilder.java](https://codecov.io/gh/apache/rocketmq/pull/4756/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bG9nZ2luZy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbG9nZ2luZy9pbm5lci9Mb2dnaW5nQnVpbGRlci5qYXZh) | `64.71% <0.00%> (+1.26%)` | :arrow_up: |
   | [...mq/store/ha/autoswitch/AutoSwitchHAConnection.java](https://codecov.io/gh/apache/rocketmq/pull/4756/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL2F1dG9zd2l0Y2gvQXV0b1N3aXRjaEhBQ29ubmVjdGlvbi5qYXZh) | `74.93% <0.00%> (+1.36%)` | :arrow_up: |
   | ... and [15 more](https://codecov.io/gh/apache/rocketmq/pull/4756/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] coveralls commented on pull request #4756: [ISSUE #4755] Some improvement about branch management

Posted by GitBox <gi...@apache.org>.

coveralls commented on PR #4756:
URL: https://github.com/apache/rocketmq/pull/4756#issuecomment-1201207161

   
   [![Coverage Status](https://coveralls.io/builds/51310504/badge)](https://coveralls.io/builds/51310504)
   
   Coverage increased (+0.04%) to 48.899% when pulling **8248539a517bff6e713357cc80b7515412711edf on aaron-ai:pr** into **6a95661291a7a94e55b6813d5298b58a779aea2b on apache:develop**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org