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/03/08 11:04:27 UTC
[GitHub] [rocketmq] ferrirW opened a new pull request #3946: ROCKETMQ-2870 move all reset topic with namespace to sendDefaultImpl()
ferrirW opened a new pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946
[](https://user-images.githubusercontent.com/42178996/157219185-70b622c4-e9d9-471d-9eaa-6404669990f2.png)image
#3424 delete check may cause an NPE.
A better way is that move checkMessage & msg.setTopic to defaultMQProducerImpl.send() and make check before.
--
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] ni-ze commented on pull request #3946: [#2870] move all reset topic with namespace to sendDefaultImpl()
Posted by GitBox <gi...@apache.org>.
ni-ze commented on pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946#issuecomment-1062466832
add namespace into topic when check message is not a good idea.
--
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] ferrirW edited a comment on pull request #3946: [#2870] move all reset topic with namespace to sendDefaultImpl()
Posted by GitBox <gi...@apache.org>.
ferrirW edited a comment on pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946#issuecomment-1062502536
> add namespace into topic when check message is not a good idea.
Whether put msg.setTopic after check message is a better way？ @ni-ze
--
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] ferrirW closed pull request #3946: [#2870] move all reset topic with namespace to sendDefaultImpl()
Posted by GitBox <gi...@apache.org>.
ferrirW closed pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946
--
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] ferrirW commented on pull request #3946: [#2870] move all reset topic with namespace to sendDefaultImpl()
Posted by GitBox <gi...@apache.org>.
ferrirW commented on pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946#issuecomment-1062502536
> add namespace into topic when check message is not a good idea.
Whether put msg.setTopic after check message is a better way？
--
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] Git-Yang commented on pull request #3946: [#2870] move all reset topic with namespace to sendDefaultImpl()
Posted by GitBox <gi...@apache.org>.
Git-Yang commented on pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946#issuecomment-1061712002
Validators.checkMessage is duplicated.
--
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] ferrirW edited a comment on pull request #3946: [#2870] move all reset topic with namespace to sendDefaultImpl()
Posted by GitBox <gi...@apache.org>.
ferrirW edited a comment on pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946#issuecomment-1062496307
> Validators.checkMessage is duplicated.
We can see git history in above screenshot that duplicate checks are used to prevent Null point exception, and it seem's necessary. @Git-Yang
--
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] ferrirW commented on pull request #3946: [#2870] move all reset topic with namespace to sendDefaultImpl()
Posted by GitBox <gi...@apache.org>.
ferrirW commented on pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946#issuecomment-1062496307
> Validators.checkMessage is duplicated.
We can see git history in above screenshot that duplicate checks are used to prevent Null point exception, and it seem's necessary.
--
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] ferrirW commented on pull request #3946: ROCKETMQ-2870 move all reset topic with namespace to sendDefaultImpl()
Posted by GitBox <gi...@apache.org>.
ferrirW commented on pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946#issuecomment-1061678986
Another simple way is move all msg.setTopic() to Validators.checkMessage(). Does it fit？
--
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] ni-ze commented on pull request #3946: [#2870] move all reset topic with namespace to sendDefaultImpl()
Posted by GitBox <gi...@apache.org>.
ni-ze commented on pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946#issuecomment-1064714042
> > add namespace into topic when check message is not a good idea.
>
> Whether put msg.setTopic after check message is a better way？ @ni-ze
Do not modify it is better, I think.
--
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] ferrirW commented on pull request #3946: [#2870] move all reset topic with namespace to sendDefaultImpl()
Posted by GitBox <gi...@apache.org>.
ferrirW commented on pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946#issuecomment-1062499708
> add namespace into topic when check message is not a good idea.
yes, it's not best way, but is the simplest change, and I have coverd all invoked of checkMessage().
--
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 #3946: [#2870] move all reset topic with namespace to sendDefaultImpl()
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3946:
URL: https://github.com/apache/rocketmq/pull/3946#issuecomment-1061833300
--
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