-
Notifications
You must be signed in to change notification settings - Fork 18
[MSHARED-1128] Update documentation with new execute method #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
michael-o
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also agree with the suggested changes from @kwin
b351fa2 to
bc00c95
Compare
|
suggestions applied, please next round of review |
michael-o
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit, but the rest looks good.
Co-authored-by: Konrad Windszus <[email protected]>
bc00c95 to
a6cb051
Compare
michael-o
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement
gnodet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, I've seen a few other deprecated methods which are not flagged as @Deprecated with a proper annotation (just the @deprecated javadoc tag):
resetStreams()displayStreamBuffers()setDebug()setMavenDebug()
It would be nice to add the annotation on those too !
|
@gnodet thanks - you are right - I will add all missing annotation in separate PR. |
gnodet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but please add @Deprecated annotations in a separate PR.
|
Resolve #155 |
1 similar comment
|
Resolve #155 |
No description provided.