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

docs: add section for Relationship Loading Strategy #588

Merged
merged 9 commits into from
May 27, 2020
Merged

docs: add section for Relationship Loading Strategy #588

merged 9 commits into from
May 27, 2020

Conversation

willsoto
Copy link
Contributor

@willsoto willsoto commented May 27, 2020

First pass. Open to feedback of course.

#440

Signed-off-by: Will Soto <willsoto@users.noreply.github.com>
docs/docs/loading-strategy.md Outdated Show resolved Hide resolved
docs/docs/loading-strategy.md Outdated Show resolved Hide resolved
docs/docs/loading-strategy.md Outdated Show resolved Hide resolved
docs/docs/loading-strategy.md Outdated Show resolved Hide resolved
docs/docs/loading-strategy.md Outdated Show resolved Hide resolved
docs/docs/loading-strategy.md Outdated Show resolved Hide resolved
willsoto and others added 4 commits May 27, 2020 07:22
Co-authored-by: Martin Adámek <banan23@gmail.com>
Co-authored-by: Martin Adámek <banan23@gmail.com>
Co-authored-by: Martin Adámek <banan23@gmail.com>
Signed-off-by: Will Soto <willsoto@users.noreply.github.com>
@willsoto
Copy link
Contributor Author

willsoto commented May 27, 2020

Feedback applied.

One thing I was thinking was whether or not we should have a section dedicated to describing all the various strategies. I know only 2 are supported now, but if there are plans to support more in the future it could be useful to have them all described up-front. I'm thinking something like this

@B4nan
Copy link
Member

B4nan commented May 27, 2020

But that is the page you just added, isn't it? It's called loading strategy, we have 2 of those, and it describes both of them. Sure we could be more verbose if that is what you mean (feel free to do so :])

@B4nan
Copy link
Member

B4nan commented May 27, 2020

Maybe we should rename the page to loading strategies? WDYT?

@willsoto
Copy link
Contributor Author

But that is the page you just added, isn't it? It's called loading strategy, we have 2 of those, and it describes both of them. Sure we could be more verbose if that is what you mean (feel free to do so :])

haha I suppose verbosity is one way to describe what I want. Yes, you are right that it is all there already but I am thinking more in the case that I already know about the feature, but I just need to look up the various supported strategies at a glance. Could be useful, but it was just a thought. I'll think on it more.

Maybe we should rename the page to loading strategies? WDYT?

I don't have a strong preference either way. I titled it Relationship Loading Strategies because I think there are a whole lot of things one can "load" via an ORM, right? This document specifically refers to how relationships get loaded. But that is certainly debatable :)

@B4nan
Copy link
Member

B4nan commented May 27, 2020

I don't have a strong preference either way. I titled it Relationship Loading Strategies because I think there are a whole lot of things one can "load" via an ORM, right? This document specifically refers to how relationships get loaded. But that is certainly debatable :)

The name is fine, what I meant is to use Loading Strategies instead of the current Loading Strategy, so just singular -> plural :]

@willsoto
Copy link
Contributor Author

I don't have a strong preference either way. I titled it Relationship Loading Strategies because I think there are a whole lot of things one can "load" via an ORM, right? This document specifically refers to how relationships get loaded. But that is certainly debatable :)

The name is fine, what I meant is to use Loading Strategies instead of the current Loading Strategy, so just singular -> plural :]

Oh! Got it haha

Sure that is fine I think, I named it singular since most (all?) of the titles are singular (like Naming Strategy). But yeah I can make that change np

@willsoto
Copy link
Contributor Author

willsoto commented May 27, 2020

Point taken. Will change.

@B4nan do you want page title and sidebar title changed?

@B4nan
Copy link
Member

B4nan commented May 27, 2020

Yes please, change it on all places, including the file name (and therefore url).

@willsoto
Copy link
Contributor Author

Should be good, I just did it via the github UI, I hope you plan on squash merging this too 😅

@B4nan
Copy link
Member

B4nan commented May 27, 2020

Sure, no worries, I am squash merging everything (I even accidentally squash merged the v3 PR so the history is totally fucked up with v3 changes being in single commit 🤣)

@willsoto
Copy link
Contributor Author

@B4nan I'm okay not adding that section for right now. You are free to merge this unless there are other changes you'd like made.

@B4nan B4nan changed the title docs(loading-strategy): add section for Relationship Loading Strategy docs: add section for Relationship Loading Strategy May 27, 2020
@B4nan B4nan merged commit 7ca8c3a into mikro-orm:dev May 27, 2020
@B4nan
Copy link
Member

B4nan commented May 27, 2020

It's good to have it there, thanks! :]

B4nan pushed a commit that referenced this pull request Jun 1, 2020
B4nan pushed a commit that referenced this pull request Jun 5, 2020
B4nan pushed a commit that referenced this pull request Jun 16, 2020
B4nan pushed a commit that referenced this pull request Aug 2, 2020
B4nan pushed a commit that referenced this pull request Aug 9, 2020
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.

None yet

2 participants