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

Remove the use of load(on:) in relations.md #956

Merged
merged 3 commits into from
Jan 2, 2024
Merged

Conversation

wojexe
Copy link
Contributor

@wojexe wojexe commented Dec 17, 2023

As I've noticed on a discord conversation, the load(on:) method is discouraged to be used and should be replaced with get(on:), which defaults to cache if possible.

@gwynne gwynne added invalid code Code sample not working. no-translation-needed This PR does not require the translations to be updated (e.g. fixing a typo or infrastructure work) labels Dec 17, 2023
@gwynne
Copy link
Member

gwynne commented Dec 17, 2023

Great! Can you also add mention (and possibly an example) of get()'s reload parameter? (The full signature is get(reload:on:), with reload defaulting to false.)

@gwynne gwynne added updates Adding new or updated documentation. and removed no-translation-needed This PR does not require the translations to be updated (e.g. fixing a typo or infrastructure work) labels Dec 17, 2023
@wojexe
Copy link
Contributor Author

wojexe commented Dec 17, 2023

Actually there is a mention of the reload: parameter higher up, in the #get section. I just thought that get(on:) would be sufficient for this use case. But yeah, on the second thought, people might want to have their data reloaded during eager loading, so I'll include it. 😄

Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

LGTM - given the additional text we'll need translations for this so I'll remove the no-translation-needed label

@0xTim 0xTim enabled auto-merge (squash) January 2, 2024 16:54
@0xTim 0xTim merged commit c9dca28 into vapor:main Jan 2, 2024
1 check passed
@penny-for-vapor penny-for-vapor bot mentioned this pull request Jan 2, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid code Code sample not working. updates Adding new or updated documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants