fix GLI usage breaking faraday >= 2.7.0 #434
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
blowfishpro
force-pushed
the
FixFaradayIncompatibility
branch
from
be86f7a to
f41c529
Compare
dblock
requested changes
Nov 27, 2022
|
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. |
Generated by 🚫 Danger |
dblock
requested changes
Nov 28, 2022
Only CHANGELOG. You can leave that alone for now. |
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
blowfishpro
force-pushed
the
FixFaradayIncompatibility
branch
from
f41c529 to
1ab48c2
Compare
|
Fixed! |
|
FYI I decided to rename |
|
I'm pretty indifferent on that particular point as long as it's consistent. |
This was referenced Feb 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
when using include at the top level, the module gets included in the Object class
Faraday::Middlewarechecks whether it responds toon_error, which then finds GLI'son_error, which has a different signature and meaningthis resolves that by wrapping the entire CLI in a class so that
GLI::Appis not included inObjectwe also use GLI's built-in
commands_frommethod to avoid having to reference every command file individuallyResolves #433
Note: The
slack:web:api:updatetask 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