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

add admin web api methods #305

Merged
merged 2 commits into from Dec 28, 2019
Merged

add admin web api methods #305

merged 2 commits into from Dec 28, 2019

Conversation

manuelmeurer
Copy link
Collaborator

I updated the API methods via rake slack:api:update, which added lots of new admin methods.
Once those are merged, we can remove the undocumented Slack::Web::Api::Endpoints::UsersAdmin.users_admin_invite

@manuelmeurer
Copy link
Collaborator Author

I changed a few small things in the changelog:

  • the last entry referenced a wrong PR number (302 instead of 303) and misspelled the added endpoint (views.published instead of views.publish)
  • most of the entries used past tense ("Added") and only a few present tense ("Add"). I changed the latter ones to past tense for consistency.

@dblock
Copy link
Collaborator

dblock commented Dec 4, 2019

This looks good, but there seem to be legit spec failures? I haven't looked any deeper than that.

@manuelmeurer
Copy link
Collaborator Author

Ok, I'll investigate!

Why does Ruby 2.4.1 on Travis only run bundle exec danger, not the actual specs?
https://travis-ci.org/slack-ruby/slack-ruby-client/jobs/620311621

@manuelmeurer
Copy link
Collaborator Author

manuelmeurer commented Dec 5, 2019

I get the same failures locally when I run the specs on master, so I don't think they are related to the changes in this PR.
I'll try to find out what's going on anyways. :)

@dblock
Copy link
Collaborator

dblock commented Dec 5, 2019

Ok, I'll investigate!

Why does Ruby 2.4.1 on Travis only run bundle exec danger, not the actual specs?
https://travis-ci.org/slack-ruby/slack-ruby-client/jobs/620311621

Danger is what gives us changelog checking and stuff like that.

@dblock
Copy link
Collaborator

dblock commented Dec 5, 2019

I get the same failures locally when I run the specs on master, so I don't think they are related to the changes in this PR.
I'll try to find out what's going on anyways. :)

Yes thank you, we want that fixed first.

@manuelmeurer
Copy link
Collaborator Author

manuelmeurer commented Dec 18, 2019

@dblock The culprit is Faraday. They are preparing for a v1.0 release and (unintentionally?) introduced some breaking changes from 0.17.0 to 0.17.1.

Basically, in 0.17.0, Faraday::Error inherits from StandardError, so we have to add the possibility to initialize a SlackError with a response.

But in 0.17.1, Faraday::Error already does this itself already.

What to do? I added a check for the arity of Faraday::Error#initialize but I think it's ugly and brittle.
Alternatively, we could limit Faraday to either < 0.17.0 or 0.17.1.

@dblock
Copy link
Collaborator

dblock commented Dec 18, 2019

Are they releasing 0.17.2 fixing the regression? Otherwise make our code compatible with 0.17.1 and add a >= 0.17.1 rather than limit below. We want to be compatible with the latest moving forward.

@manuelmeurer
Copy link
Collaborator Author

I opened an issue in the Faraday repo and will add a limit to >= 0.17.1 for now.

@manuelmeurer
Copy link
Collaborator Author

@dblock After the Faraday issue was fixed in #309, this should be good to go!

@dblock dblock merged commit 40cf30c into slack-ruby:master Dec 28, 2019
@manuelmeurer manuelmeurer deleted the update-web-api branch December 29, 2019 11:28
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