Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

codenamev
Copy link
Member

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.

Copy link
Contributor

@benjaminoakes benjaminoakes left a 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?

Comment on lines +8 to +20
# @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
Copy link
Contributor

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?

Suggested 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

Screenshot 2024-02-13 at 13 15 23

Comment on lines +10 to +11
warn "DEPRECATION WARNING: the class SuperSpreader::StopSignal is deprecated and will be removed in v1.0. " \
"Use TrackBallast::StopSignal instead."
Copy link
Contributor

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.)

@benjaminoakes benjaminoakes self-assigned this Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants