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

rename: respond_to? ->stubba_respond_to? #742

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nitishr
Copy link
Contributor

@nitishr nitishr commented Feb 20, 2025

Enhances the clarity of the code by explicitly indicating that the method is related to the stub functionality, which aids in understanding the code's intent.

Key Changes:

  1. Method Existence Checks:
    • Renamed ClassMethods::AnyInstance#respond_to? to ClassMethods::AnyInstance#stubba_respond_to? to better reflect its purpose as part of a mocha contract.
    • Mockery#on_stubbing now uses stubba_respond_to? instead of the generic respond_to? to check if an object responds to a method before stubbing it.
    • Added ObjectMethods#stubba_respond_to? to provide a consistent interface for checking if an object responds to a method.
  2. RuboCop Configuration Updates: Moved test-specific RuboCop rules from the global .rubocop.yml to a new test/.rubocop.yml file (as suggested in commits e423977, 33e373c, fddebc3).

Closes #713.

@nitishr nitishr force-pushed the dont-override-respond_to branch from 6709526 to d742fb6 Compare February 20, 2025 12:47
Rename respond_to? to stubba_respond_to? to better reflect its purpose
as part of a mocha contract.

Mockery#on_stubbing now uses stubba_respond_to? instead of the generic
respond_to? to check if an object responds to a method before stubbing
it.

Added ObjectMethods#stubba_respond_to? to provide a consistent interface
for checking if an object responds to a method.
@nitishr nitishr force-pushed the dont-override-respond_to branch from 631f246 to 0206af7 Compare February 20, 2025 20:58
@floehopper
Copy link
Member

Many thanks for taking the time to submit this PR and the other ones!

Enhances the clarity of the code by explicitly indicating that the method is related to the stub functionality, which aids in understanding the code's intent.

Key Changes:

1. **Method Existence Checks:**
   
   * Renamed `ClassMethods::AnyInstance#respond_to?`  to `ClassMethods::AnyInstance#stubba_respond_to?` to better reflect its purpose as part of a mocha contract.

While I'm not vehemently opposed to the new name in the short term, I'm keen to move away from the use of "stubba" all together in the longer term as per #360. However, the new name probably does make sense in the context of the current code. I just wanted to flag this longer-term desire!

   * `Mockery#on_stubbing` now uses `stubba_respond_to?` instead of the generic `respond_to?` to check if an object responds to a method before stubbing it.
   * Added `ObjectMethods#stubba_respond_to?` to provide a consistent interface for checking if an object responds to a method.

Can you explain a bit about why you think this approach is better than definingClassMethods::AnyInstance#respond_to_missing? instead of #respond_to? as suggested in #713? It might well be better, I've just lost track of why I wrote #713 the way I did!

2. **RuboCop Configuration Updates:** Moved test-specific RuboCop rules from the global .rubocop.yml to a new test/.rubocop.yml file (as suggested in commits [e423977](https://github.com/freerange/mocha/commit/e423977cc9fa6cfc4389252ca34342192891e031), [33e373c](https://github.com/freerange/mocha/commit/33e373ca5aa69061124df72e848f63850214f06d), [fddebc3](https://github.com/freerange/mocha/commit/fddebc3815fe3bba69b24cedc575720196f7c4b6)).

I might have missed someting, bit the changes in point 2 feel somewhat orthogonal to the changes in point 1. If so, I'd prefer to have them in a separate PR.

As stated in commits e423977, 33e373c, fddebc3:

Ideally I would've configured the extra `AllowedMethods` to only apply
to the tests, but I couldn't find a simple way to do that with the
rubocop configuration. It might be worth extracting a separate rubocop
configuration for the tests.

Here we do just that.
@nitishr nitishr force-pushed the dont-override-respond_to branch from 0206af7 to 0f33921 Compare February 21, 2025 17:19
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.

Investigate whether Mocha::ClassMethods::AnyInstance should be overriding #respond_to?
2 participants