Skip to content

Conversation

ericproulx
Copy link
Contributor

Remove Settings Unset Functions

Summary

This PR removes several internal "unset" functions from the Grape DSL settings module that were not part of the public API and simplifies the codebase by consolidating functionality and improving maintainability.

Changes Made

Removed Functions from Grape::DSL::Settings

  • unset(type, key) - Generic unset method for deleting keys from settings
  • unset_namespace_stackable(*keys) - Convenience method for unsetting multiple namespace stackable keys
  • namespace_inheritable_to_nil(key) - Method for setting namespace inheritable values to nil
  • namespace_reverse_stackable_with_hash(key) - Method for getting namespace reverse stackable values as a hash

Code Consolidation

1. Simplified reset_validations! method

  • Before: Called unset_namespace_stackable(:declared_params, :params, :validations)
  • After: Directly calls inheritable_setting.namespace_stackable.delete(:declared_params, :params, :validations)
  • Benefit: Eliminates unnecessary method delegation and makes the code more direct

2. Enhanced BaseInheritable#delete method

  • Before: delete(key) - only accepted single key
  • After: delete(*keys) - accepts multiple keys and returns array of deleted values
  • Benefit: More flexible and efficient for bulk operations

3. Refactored without_root_prefix_and_versioning method

  • Before: Used multiple namespace_inheritable_to_nil calls and manual restoration
  • After: Uses direct hash operations with proper cleanup in ensure block
  • Benefit: More robust error handling and cleaner code structure

4. Moved namespace_reverse_stackable_with_hash logic

  • Before: Public method in Grape::DSL::Settings
  • After: Private method rescue_handlers in Grape::Endpoint (only place it was used)
  • Benefit: Reduces public API surface and moves functionality closer to where it's used

Test Updates

  • Removed tests for deleted unset and unset_namespace_stackable methods
  • Updated reset_validations! test to verify actual behavior instead of method calls
  • Enhanced test setup to properly initialize namespace stackable values

Benefits

  1. Reduced API Surface: Removes internal methods that weren't part of the public API
  2. Improved Maintainability: Consolidates functionality and reduces method delegation
  3. Better Error Handling: The without_root_prefix_and_versioning method now uses ensure blocks for proper cleanup
  4. Enhanced Flexibility: The delete method now supports multiple keys
  5. Cleaner Code: Direct method calls instead of unnecessary wrapper methods

Breaking Changes

None - All removed methods were internal and not part of the public API.

Testing

  • All existing tests pass
  • New tests verify the actual behavior of reset_validations! instead of just method calls
  • Enhanced test coverage for the updated functionality

Related Issues

This cleanup is part of ongoing efforts to simplify the Grape codebase and remove unnecessary internal APIs that were not intended for public use.

@ericproulx ericproulx marked this pull request as ready for review September 20, 2025 11:53
@ericproulx ericproulx requested a review from dblock September 20, 2025 11:53
- Remove unset, unset_namespace_stackable, and namespace_inheritable_to_nil methods
- Simplify reset_validations! to call delete directly instead of through wrappers
- Enhance BaseInheritable#delete to accept multiple keys
- Improve without_root_prefix_and_versioning with proper ensure block cleanup
- Update tests to verify actual behavior instead of method calls

These methods were internal APIs not intended for public use. Removing them
reduces the public API surface and simplifies the codebase.
@ericproulx ericproulx force-pushed the remove_settings_unset_functions branch from a5b1a22 to c947351 Compare September 20, 2025 12:05
@dblock dblock merged commit e35d292 into master Sep 21, 2025
93 checks passed
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