feat/joinable-task #403
Merged
feat/joinable-task #403
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
killme2008
reviewed
Mar 20, 2020
| @@ -199,14 +199,14 @@ public void setErrorMsg(String errMsg) { | |||
| * Set error code and error msg. | |||
| */ | |||
| public void setError(int code, String fmt, Object... args) { | |||
| this.state = new State(code, String.format(fmt, args)); | |||
| this.state = new State(code, String.format(String.valueOf(fmt), args)); | |||
There was a problem hiding this comment.
fmt 已经是字符串了，这里为什么还要 valueOf
There was a problem hiding this comment.
fmt 有可能为 Null 导致 NPE
There was a problem hiding this comment.
比如 jraft 有一些这样的调用 setError(-1, e.getMessage()); 其中 e.getMessage() == null
killme2008
reviewed
Mar 20, 2020
| * @since 1.3.1 | ||
| */ | ||
| public Closure join() throws InterruptedException { | ||
| final JoinableClosure joinable = toJoinalbe(this.done); |
There was a problem hiding this comment.
是不是可以保存一个 toJoinable 的结果到 field，复用
There was a problem hiding this comment.
应该是 method name 起的不好误导了你，修改为 castToJoinalbe ，这里是一个类型检查和类型转换，不太有必要保存复用
killme2008
approved these changes
Mar 25, 2020
zongtanghu
pushed a commit
that referenced
this pull request
Apr 28, 2020
* joinable for raft task * fix: fix the fmt NPE * code format * better name * typo
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
For nacos's advice, add join method for Task（raft）,
and fix a NPE bug.
Modification:
Result:
Fixes #404