search

slack-ruby / slack-ruby-client

A Ruby and command-line client for the Slack Web, Real Time Messaging and Event APIs.
MIT License
1.21k stars 216 forks source link

fix GLI usage breaking faraday >= 2.7.0 #434

Closed blowfishpro closed 2 years ago

blowfishpro commented 2 years ago

when using include at the top level, the module gets included in the Object class

Faraday::Middleware checks whether it responds to on_error, which then finds GLI's on_error, which has a different signature and meaning

this resolves that by wrapping the entire CLI in a class so that GLI::App is not included in Object

we also use GLI's built-in commands_from method to avoid having to reference every command file individually

Resolves #433

Note: The slack:web:api:update task seemed to want to pull in some new stuff - since that usually seems to be done separately I have intentionally excluded that but can include it if you want

blowfishpro commented 2 years ago

That's what I did, I just did it while the code to update the submodule was commented out.

What documentation updates are required when new API features are pulled in? I don't really understand the scope of changes that come with updating the submodule.

dangerpr-bot commented 2 years ago
1 Warning
:warning: There're library changes, but not tests. That's OK as long as you're refactoring existing code.

Generated by :no_entry_sign: Danger

dblock commented 2 years ago

What documentation updates are required when new API features are pulled in? I don't really understand the scope of changes that come with updating the submodule.

Only CHANGELOG. You can leave that alone for now.

blowfishpro commented 2 years ago

Fixed!

dblock commented 2 years ago

FYI I decided to rename CLI to Cli in https://github.com/slack-ruby/slack-ruby-client/pull/435. LMK if you think it's a bad idea.

blowfishpro commented 2 years ago

I'm pretty indifferent on that particular point as long as it's consistent.