Deprecated state getter setters in behavior builder and its IMPL #1541
Conversation
|
@renatocaval @erip @TimMoore |
There was a problem hiding this comment.
Thank you, @ayushprashar.
We should just fill the deprecated message and since, but besides that, it looks good to me.
| @@ -211,8 +211,10 @@ abstract class PersistentEntity[Command, Event, State] { | |||
| private var commandHandlers: Map[Class[_ <: Command], JBiFunction[_ <: Command, CommandContext[Any], Persist[_ <: Event]]] = | |||
| cmdHandlers | |||
|
|
|||
| @scala.deprecated | |||
There was a problem hiding this comment.
Add message and since to the deprecation annotations. For example:
@deprecated("A message explaining why this method was deprecated", since = "1.5.0")This way, the compiler can give users better warning messages and directions about how to avoid the deprecated APIs.
There was a problem hiding this comment.
@marcospereira Done! Please let me know for any further changes.
|
@renatocaval @marcospereira |
|
@ayushprashar, yeah, maybe one more small thing: I was thinking about the sentence on the deprecation message: I don't know if unproductive is the right term here. If I read back the comment that triggered issue #1330, in #1302 (comment) I believe that the main reason for deprecating it is that we don't really have a reason for this to exist. It's probably a letfover from some previous refactoring. Maybe we should simply say that the method is The method is deprecated because it was deemed obsolete. |
|
Very well @renatocaval. Agreed. |
|
Done! @renatocaval @marcospereira |
|
Thanks for this @ayushprashar. And for the patience. :-) |
Pull Request Checklist
Fixes
Fixes #1330
Purpose
What does this PR do?
This PR deprecated the setState and getState methods of the behaviour builder and its impl.
Background Context
Why did you take this approach?
References
Are there any relevant issues / PRs / mailing lists discussions?
#1302