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

[9.x] Include Eloquent Model Observers in model:show command #43470

Closed
wants to merge 4 commits into from

Conversation

mike-healy
Copy link
Contributor

@mike-healy mike-healy commented Jul 29, 2022

This PR builds on the new artisan model:show command to also list Model Observers. They could be handy to see on the Model information as it's not always obvious that a Model is being observed.

It handles cases where multiple observers are attached to an action.

console output listing Model Observers

@driesvints driesvints requested a review from jessarcher July 29, 2022 08:16
@jbrooksuk jbrooksuk changed the title Include Eloquent Model Observers in model:show command [9.x] Include Eloquent Model Observers in model:show command Jul 29, 2022
@taylorotwell
Copy link
Member

I kinda want to let this command settle a bit before I start adding much more stuff to it. Let's circle back to this in a few weeks.

@mike-healy
Copy link
Contributor Author

mike-healy commented Sep 2, 2022

Hey @taylorotwell, what's the feeling like now with the model:show command?

Given the command tells us most other things about a model I think including observers would fit its purpose pretty well. They can also be less discoverable as you can't always see that observers exist by looking at the model file.

@mike-healy
Copy link
Contributor Author

Hey @taylorotwell, just bumping this now that model:show has had some time to sit.
Can we include Model Observer info in the output?

@mike-healy
Copy link
Contributor Author

Any thoughts on this @timacdonald?

I know Model Observers aren't used that often, but IMO that's a good reason to display them. Might make debugging an issue easier if a maintainer wasn't considering them.

@jessarcher
Copy link
Member

I'm keen on this feature and think it makes a lot of sense!

@@ -339,6 +376,17 @@ protected function displayCli($class, $database, $table, $attributes, $relations

$this->newLine();

$this->components->twoColumnDetail('<fg=green;options=bold>Eloquent Observers</>');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably just make this "Observers". I'm also thinking it should go beneath "Relations"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Jess. I've made those changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jessarcher Ahoy hoy. Any more to do on this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mike-healy I don't see changes. Do they not show because it's closed?

Copy link
Contributor Author

@mike-healy mike-healy Nov 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jessarcher I think that might be it.
I made that change in a29981, but it didn't seem to update the PR.

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