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

documentation on default relationship models #3404

Merged
merged 1 commit into from
Jul 2, 2017
Merged

documentation on default relationship models #3404

merged 1 commit into from
Jul 2, 2017

Conversation

browner12
Copy link
Contributor

in reference to laravel/framework#19733

@decadence
Copy link
Contributor

What's about HasOne? Can you add note that withDefault is available for it? Or it is already somewhere in the docs?

@decadence
Copy link
Contributor

Yep, there is no any mention about withDefault in docs.

@browner12
Copy link
Contributor Author

@taylorotwell thoughts on this? I didn't want to duplicate documenting the withDefault method under every relation it is available in.

What if I made a top level heading at the very bottom (under 'Touching Parent Timestamps') that could be a more general description of it, and which relations it was available for? @sileence is working on it for some of the other relations right now, so then we could add them to the list when he's done.

@sileence
Copy link
Contributor

It'd make sense to document the feature and mention the relations that support it. By the way here is the PR for morphOne laravel/framework#19788.

@taylorotwell
Copy link
Member

I don't really mind duplicating a basic example of it under both sections. I can tolerate duplication more in docs than code because it's more of a user assistance thing.

@taylorotwell taylorotwell merged commit e4ea782 into laravel:5.4 Jul 2, 2017
@browner12 browner12 deleted the belongsto-with-default branch July 2, 2017 14:47
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.

4 participants