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

Check for Deprecated on method page #55

Merged
merged 1 commit into from
Oct 2, 2022

Conversation

kstole
Copy link
Collaborator

@kstole kstole commented Oct 1, 2022

#48 updated the spiders to grab methods from JSON data included on the Web API methods page. As noted in this comment, the deprecation info in that data can be inaccurate.

This change continues using that deprecation data but also checks for an individual method reference page labeling the method deprecated (as is the case for rtm.start).

@kstole
Copy link
Collaborator Author

kstole commented Oct 1, 2022

@dblock It’s been quite a while since I contributed to the Slack gems. I just haven’t been working on Slack bots the last few years. But I recently had to patch our company’s bot to work with rtm.connect so I’m drafting a few PRs to update that in the main gems as well. I’m also participating in Hacktoberfest so it would be great if you can add the hacktoberfest-accepted label on those PRs and maybe hacktoberfest on the overall repos could encourage some participation. Thanks!

@dblock
Copy link
Collaborator

dblock commented Oct 2, 2022

Thanks @kstole! Want to help me maintain this repo? Then you can create some hacktoberfest labels :)

Copy link
Collaborator

@dblock dblock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is right. We collect deprecated methods and mark them deprecated: true in the JSON. This skips them?

@@ -36,7 +36,9 @@ def process_list(page, _default_data = {})

def process_method(page, default_data = {})
method_page = ensure!(page, '.apiMethodPage', default_data[:method_name])
desc = method_page.search('.apiReference__mainDescription').text.gsub("’", "'")
desc = method_page.search('.apiReference__mainDescription').text.gsub('’', "'")
return if desc.downcase.start_with? 'deprecated:'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this change the value of deprecated: below instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like that used to be the behavior but was changed in #48 to hardcode 'deprecated' => false and just remove any deprecated methods, so this is an extra check for when isDeprecated isn't accurate

next if method['isDeprecated']

@dblock
Copy link
Collaborator

dblock commented Oct 2, 2022

Consider separating the " changes into a separate PR that would enable rubocop and do a pass with that on the repo code with -a.

@kstole
Copy link
Collaborator Author

kstole commented Oct 2, 2022

Thanks @kstole! Want to help me maintain this repo? Then you can create some hacktoberfest labels :)

Yeah for sure. Happy to help maintain

@dblock
Copy link
Collaborator

dblock commented Oct 2, 2022

Thanks @kstole! Want to help me maintain this repo? Then you can create some hacktoberfest labels :)

Yeah for sure. Happy to help maintain

Added you, https://github.com/slack-ruby/slack-api-ref/invitations. Please continue making PRs, but feel free to merge others' and make labels as you please.

@dblock dblock merged commit 791d0f5 into slack-ruby:master Oct 2, 2022
@kstole kstole deleted the deprecated_desc branch October 2, 2022 03:55
dblock added a commit that referenced this pull request Oct 3, 2022
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.

None yet

2 participants