-
Notifications
You must be signed in to change notification settings - Fork 1
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
Deprecate SuperSpreader::StopSignal and related methods #69
base: master
Are you sure you want to change the base?
Conversation
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.
Thank you! Great call on using the @deprecated
feature. ❤️
To save us from making another PR, I would suggest that we bump the version (0.2.2?), add a deprecation notice to the CHANGELOG, and release a new version as a part of merging this PR. This would give any possible users of SuperSpreader::StopSignal
some time to become aware of this change while we continue working on doximity/track_ballast#19.
What do you think?
# @deprecated Please use {TrackBallast::StopSignal.stop!} instead | ||
def stop! | ||
warn "DEPRECATION WARNING: the class SuperSpreader::StopSignal is deprecated and will be removed in v1.0. " \ | ||
"Use TrackBallast::StopSignal instead." | ||
redis.set(stop_key, true) | ||
end | ||
|
||
# @deprecated Please use {TrackBallast::StopSignal.go!} instead | ||
def go! | ||
redis.del(stop_key) | ||
end | ||
|
||
# @deprecated Please use {TrackBallast::StopSignal.stopped?} instead |
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.
When previewing the documentation using rake doc
, these warnings appear:
[warn]: In file `lib/super_spreader/stop_signal.rb':7: Cannot resolve link to TrackBallast::StopSignal.go! from text:
...{TrackBallast::StopSignal.go!}...
[warn]: In file `lib/super_spreader/stop_signal.rb':7: Cannot resolve link to TrackBallast::StopSignal.stop! from text:
...{TrackBallast::StopSignal.stop!}...
[warn]: In file `lib/super_spreader/stop_signal.rb':7: Cannot resolve link to TrackBallast::StopSignal.stopped? from text:
...{TrackBallast::StopSignal.stopped?}...
[warn]: In file `lib/super_spreader/stop_signal.rb':16: Cannot resolve link to TrackBallast::StopSignal.go! from text:
...{TrackBallast::StopSignal.go!}...
[warn]: In file `lib/super_spreader/stop_signal.rb':9: Cannot resolve link to TrackBallast::StopSignal.stop! from text:
...{TrackBallast::StopSignal.stop!}...
[warn]: In file `lib/super_spreader/stop_signal.rb':21: Cannot resolve link to TrackBallast::StopSignal.stopped? from text:
...{TrackBallast::StopSignal.stopped?}...
It makes sense that it can't resolve the links because we are moving StopSignal
to a different gem in doximity/track_ballast#19. When the curly braces are removed, the warnings go away and the resulting documentation appears to be identical. What do you think about this change?
# @deprecated Please use {TrackBallast::StopSignal.stop!} instead | |
def stop! | |
warn "DEPRECATION WARNING: the class SuperSpreader::StopSignal is deprecated and will be removed in v1.0. " \ | |
"Use TrackBallast::StopSignal instead." | |
redis.set(stop_key, true) | |
end | |
# @deprecated Please use {TrackBallast::StopSignal.go!} instead | |
def go! | |
redis.del(stop_key) | |
end | |
# @deprecated Please use {TrackBallast::StopSignal.stopped?} instead | |
# @deprecated Please use TrackBallast::StopSignal.stop! instead | |
def stop! | |
warn "DEPRECATION WARNING: the class SuperSpreader::StopSignal is deprecated and will be removed in v1.0. " \ | |
"Use TrackBallast::StopSignal instead." | |
redis.set(stop_key, true) | |
end | |
# @deprecated Please use TrackBallast::StopSignal.go! instead | |
def go! | |
redis.del(stop_key) | |
end | |
# @deprecated Please use TrackBallast::StopSignal.stopped? instead |
warn "DEPRECATION WARNING: the class SuperSpreader::StopSignal is deprecated and will be removed in v1.0. " \ | ||
"Use TrackBallast::StopSignal instead." |
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.
There's also the built-in deprecate
method, which may provide some advantages. I'm unsure, to be honest. Do you have an opinion on this option? (I could go either way.)
This is step 1 in an effort (see #66) to port
StopSignal
to our TrackBallast library (see track_ballast#19).There will be a followup PR to officially replace it.