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

[10.x] Add Factory::getNamespace() #47463

Merged

Conversation

tylernathanreed
Copy link
Contributor

When overriding the Factory name guesser via Factory::guessFactoryNamesUsing(), I often need to know the current factory namespace. While I could certainly just hard-code Database\\Factories\\, that goes out the window if Factory::useNamespace() is used.

This PR just adds Factory::getNamespace() to return the static::$namespace property.

@tylernathanreed tylernathanreed changed the title Add Factory::getNamespace() [10.x] Add Factory::getNamespace() Jun 16, 2023
@taylorotwell taylorotwell merged commit 33afb59 into laravel:10.x Jun 16, 2023
@flap152
Copy link

flap152 commented Jun 28, 2023

Note: In the end, static::$namespace was simply made public.
Shouldn't the PR be renamed to "[10.x] Make Factory::$namespace public"?

@tylernathanreed
Copy link
Contributor Author

@flap152 I don't know if it's Taylor's practice to change PR titles. My original intent vs what got accepted are different.

@flap152
Copy link

flap152 commented Jun 28, 2023

I saw this PR through a Laravel Shift newsletter the referenced changes between releases with the title "Add getNamespace to Factory in #47463". It's also in the 10.14 Release content, issued yesterday, so renaming is probably a moot point, maybe impossible, possibly causing harm because it's been released.

I'm getting closer to contributing, looking at the process and "customs", and looking at the end result, I first wondered if there was a "magical getter" get{StaticProperty}() that turned a getNamespace() call into return static::$namespace 😅

I wonder if @taylorotwell did not notice the now-misleading title, or noticed but ignored it because he's busy, it's a small detail, etc. but not commenting on the change seemed odd to me. So the 1st line of my comment is to quickly notify other readers of the change in intent and implementation, the 2nd line is to maybe learn something.

@tylernathanreed tylernathanreed deleted the features/factory-get-namespace branch March 25, 2024 21:30
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.

3 participants