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

make:entity repository add/remove method signature #1086

Closed
JB-oclock opened this issue Apr 2, 2022 · 1 comment · Fixed by #1087
Closed

make:entity repository add/remove method signature #1086

JB-oclock opened this issue Apr 2, 2022 · 1 comment · Fixed by #1087
Labels
Bug Bug Fix HasPR Status: Reviewed Has been reviewed by a maintainer

Comments

@JB-oclock
Copy link
Contributor

Hi,

i'm happy to have this two new method: it's easier to use Repository than EntityManagerInterface in Controller.

But i'm not sure that the flush parameter with default value to true is a good choice.

public function add(<?= $entity_class_name ?> $entity, bool $flush = true): void

I see datamapper as a "transaction" use of the database, so using flush by default and not using "transaction" was not what i expect.

I just want to know your vision of the default value to true
This issue is more a discussion.

@hhamon
Copy link

hhamon commented Apr 3, 2022

But i'm not sure that the flush parameter with default value to true is a good choice.

I also agree that flushing the transaction should not be triggered implicitely. By default, the developer should ->flush() explicitely or pass true explicitely to this method.

@jrushlow jrushlow added Bug Bug Fix Status: Reviewed Has been reviewed by a maintainer HasPR labels Apr 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix HasPR Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants