-
-
Notifications
You must be signed in to change notification settings - Fork 217
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
Log when methods are deprecated #329
Conversation
Add a spec similar to undocumented methods. I think we have to decide whether the method is or will be deprecated. I would say "is". If the method stops working on a certain date we should extract that into slack-api-ref and add the message in the warning along the lines of "it will stop working on ...". |
What do you think about linking to the specific slack API instead, or copying the entire message? The deprecation warning actually has two dates e.g. https://api.slack.com/methods/channels.list
|
Even better. I would use the message as is. |
There are some new methods picked up with the updated submodule, I haven't committed them - barring failed tests, will open a separate PR for them. |
spec/slack/web/client_spec.rb
Outdated
let(:client) { described_class.new } | ||
|
||
it 'produces a warning' do | ||
expect(client.logger).to receive(:warn).with(/^channels.archive: This method is deprecated./) |
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.
Check for a list of alternate methods here.
Needs a |
# Offense count: 77 | ||
# Configuration parameters: . | ||
# SupportedStyles: have_received, receive | ||
RSpec/MessageSpies: | ||
EnforcedStyle: receive | ||
|
||
# Offense count: 94 | ||
# Offense count: 95 | ||
RSpec/MultipleExpectations: | ||
Max: 5 |
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.
EDITED
Ok I managed to get by this offence by doing this:
logger = instance_double(Logger)
allow(client).to receive(:logger).and_return(logger)
allow(client).to receive(:post)
expect(logger).to receive(:warn).with(/
^channels\.archive:\ This\ method\ is\ deprecated
.+
Alternative\ methods:\ conversations\.archive\.
/x)
client.channels_archive(channel: 'test')
Do you think I should do so? If not, could we ignore these style cops completely :D
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 think ignoring expectations is fine. I usually just -a
everything and selectively fix if I feel like it.
Merged, thank you. |
Follow-up to #320 - log when a method is / will be deprecated.
There are some new methods that were added as well from the latest
slack-api-ref
, I'll open a new PR.