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

use B for detecting methods rather than Moose #8

Merged
merged 1 commit into from
Jan 16, 2023

Conversation

haarg
Copy link
Member

@haarg haarg commented Jan 9, 2023

Rather than using Moose, and protected by a Test::Needs check, use B to check the package of subs to detect if they are methods.

Since we aren't using Moose to create anything in the test, we don't need to care about any of Moose's own tracking or other things it cares about like roles. We only are using it to check is the package name attached to the sub. This can be done directly with B. This allows dropping the hard dependency on Test::Needs, and always run this test.

Rather than using Moose, and protected by a Test::Needs check, use B to
check the package of subs to detect if they are methods.

Since we aren't using Moose to create anything in the test, we don't
need to care about any of Moose's own tracking or other things it cares
about like roles. We only are using it to check is the package name
attached to the sub. This can be done directly with B. This allows
dropping the hard dependency on Test::Needs, and always run this test.
@karenetheridge
Copy link
Member

I think the intent of using Moose in tests was that so the method was not the same as what was being done in code (which already uses B::svref_2object and checking ->STASH->NAME - if the check is being done the same way, we're not really testing anything.

@haarg
Copy link
Member Author

haarg commented Jan 16, 2023

The code checking ->STASH->NAME is only in the fresh modifier. This test isn't checking anything related to fresh, but around modifiers.

When the test was written, it was just using code from Test::CleanNamespaces to get the list of methods. At the time, Test::CleanNamespaces always used Moose to get its understanding of what a method was. I don't think that was due to any attempt to use a specific type of implementation, just what was available at the time.

In the end, for anything not installed via Moose's meta layer (which this is not doing) Moose is just doing the same check of the package name attached to the sub. So it could pretty much also be called "the same way".

I would really like for Class::Method::Modifiers to only use core prereqs. The largest user of Class::Method::Modifiers is Moo, and this and PR#7 are last extraneous prereqs in Moo's dependency tree.

@karenetheridge karenetheridge merged commit adb3039 into master Jan 16, 2023
@karenetheridge karenetheridge deleted the haarg/no-test-needs-moose branch January 16, 2023 21:15
karenetheridge added a commit that referenced this pull request Jan 16, 2023
        - remove Test::Fatal and Test::Needs from test prereqs (PRs #7, #8,
          Graham Knop)
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