-
Notifications
You must be signed in to change notification settings - Fork 480
Support Grape 3 compatibility #966
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
Conversation
|
Looks good! Add to CHANGELOG, update README for versions, write some |
|
Absolutely! As usual. Probably CI Matrix needs to be updated also. |
|
@numbata looks like legit CI failures |
- Implement robust token owner resolution from endpoint methods, helper
modules, and stackable helpers
- Add support for detecting and properly evaluating Proc arguments via
parameter inspection
- Add comprehensive test coverage for resolver behavior including:
* Direct method resolution on endpoints
* Helper module resolution via namespace stack
* Proc evaluation with and without arguments
* Error handling for undefined methods
|
@dblock yes. Working on it. The issue is that helpers also should be taken for account during resolving. |
Remove dead code and improve error handling: - Remove fetch_endpoint_helpers: Endpoints don't have a public helpers method, making this code path unreachable - Simplify resolve_from_helper: Remove non-Module branch since helpers from namespace_stackable are always Modules - Simplify gather_helpers: Only one source of helpers (stackable), remove unnecessary array consolidation - Remove __send__ usage: inheritable_setting is a public method, use direct call - Improve exception handling: Replace broad rescue StandardError with specific NoMethodError and NameError catches in fetch_stackable_helpers Error handling improvements: - Add GrapeSwagger::Errors::TokenOwnerNotFound inheriting from NoMethodError for more semantic error hierarchy - Update resolver to raise custom error instead of generic NoMethodError, allowing users to catch grape-swagger-specific issues
Grape 1.8.0 is incompatible with activesupport >= 7.2 due to deprecation API changes. Add conditional constraint to allow testing with Grape 1.8.0 while maintaining compatibility with Grape 2.x and 3.x.
887e7ce to
c0d2b72
Compare
Some Grape versions have protected inspect methods on endpoint objects. Use endpoint.class for the error message instead, which is simpler and works reliably across all Grape versions (1.8.0, 2.1.3, 2.4.0, 3.0+). This resolves failures when testing with Ruby 3.1.0 and Grape 2.1.3.
|
@dblock This looks complete now. Once this PR is released, we can officially declare support for Grape 3.0 for this gem. 🎉 |
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.
Thanks for taking care of it!
This PR replaces the
namespace_stackableaccess removed in Grape 3 withinheritable_settingusage so namespace aggregation still works, and raises the Grape dependency ceiling to< 4.0.Fix #965