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

Allow excluding directories from the model search with sensible defaults #135

Closed
wants to merge 1 commit into from

Conversation

hailwood
Copy link
Contributor

In L5 (this is also backwards compatible/non-breaking with L4) your models now live in the base app directory -
You also have a bunch of other directories at the same level that do not contain models Console, Exception, Http etc -

These generate a lot of "xxx is not a model" message, this requests allows you to set paths relative to the base url (the same as model_locations) that classes found will not be added to the models array.

This is recursive so excluding app/Console will exclude app/Console/Commands etc.

@barryvdh
Copy link
Owner

Hmm, what about just silencing those classes instead?

@hailwood
Copy link
Contributor Author

How do you propose silencing them?

@barryvdh
Copy link
Owner

} elseif (!$reflectionClass->isSubclassOf('Illuminate\Database\Eloquent\Model')) {
    $this->comment("Class '$name' is not a model");
    continue;
}

Remove the $this->comment() line?

@hailwood
Copy link
Contributor Author

Ah yes that would work too since I don't see any value in telling the user that "hey this file isn't a model so we aren't going to touch it" - that should be obvious really.

Potentially this could still be merged in with a couple of changes - change the key to 'ignored_locations', and make the path have to match the entire model path (so not recursive) for anyone that would like the ability to just ignore a whole directory of models rather than file by file.

If you don't think there is any value in that then I'll close this issue with good faith that you'll remove that comment :)

barryvdh added a commit that referenced this pull request Nov 29, 2014
Just show when it's actually a Model, not every class (for L5
especially). See #135
@barryvdh
Copy link
Owner

Hmm, I'm not really sure if it's useful, it might add some confusion/complexity. But let's see if we run in errors without it first.
I've removed some comments, it just shows the actual models now (see above commit)

@hailwood
Copy link
Contributor Author

Your commit above has removed the errors so we are good to go.

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