Skip to content

Add lookup strategy to writer - #11

Closed
ddeboer wants to merge 2 commits into
masterfrom
lookup-strategy
Closed

Add lookup strategy to writer#11
ddeboer wants to merge 2 commits into
masterfrom
lookup-strategy

Conversation

@ddeboer

@ddeboer ddeboer commented Dec 8, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

See #9 (comment).

Fix #3: if you want to use QueryBuilder, you can just implement your own LookupStrategy.

@ddeboer
ddeboer requested a review from Baachi December 8, 2017 13:28
@ddeboer ddeboer mentioned this pull request Dec 8, 2017
Closed
@Baachi

Baachi commented Dec 8, 2017

Copy link
Copy Markdown
Contributor

Wow you'r fast 😲

Good Job!

@Baachi

Baachi commented Dec 8, 2017

Copy link
Copy Markdown
Contributor

Should we add a depreciation note somewhere? So we can remove this static constructur in the next major version?

@ddeboer

ddeboer commented Dec 8, 2017

Copy link
Copy Markdown
Member Author

Added a (silenced) deprecation warning.

ddeboer
ddeboer commented Dec 8, 2017
View reviewed changes
Comment thread src/DoctrineWriter.php
*/
public function __construct(
ObjectManager $objectManager,
$objectName,

@ddeboer ddeboer Dec 8, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

$objectName needs to passed both here and when constructing a LookupStrategy. Not very elegant, but I don’t see any other options now.

@slashrsm slashrsm mentioned this pull request Jul 23, 2026
Merged
5 tasks
@slashrsm

slashrsm commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Superseded by a 2.x port: #31

The original branch cannot be rebased cleanly onto current master (CONFLICTING; PHP 8.2 / doctrine/persistence modernization). PR #31 reimplements the LookupStrategy idea with BC-safe defaults and fixes several issues in this branch (missing withIndex, clone mutation in withLookupFields, factory arg order / type mismatch, null lookup not creating entities).

Recommendation: close this PR once #31 is reviewed (do not merge this 2017 branch).

slashrsm added a commit that referenced this pull request Jul 23, 2026
@slashrsm
Add LookupStrategy to DoctrineWriter (port of #11 for 2.x) (#31)
f539cf8 
Additive 2.x port of lookup strategies with tests; BC defaults preserved.
@slashrsm slashrsm closed this in #31 Jul 23, 2026
@slashrsm

slashrsm commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Superseded by #31 (merged), which reimplements lookup strategies for the 2.x line with tests and green CI.

The 2017 branch could not be rebased cleanly; not merged as-is. Thanks for the original design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Baachi Baachi Baachi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ddeboer @Baachi @slashrsm