Skip to content

[DoctrineBridge] Fix bug when indexBy is meta key in PropertyInfo\DoctrineExtractor #25841

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

Merged
merged 1 commit into from
Apr 20, 2018

Conversation

insekticid
Copy link
Contributor

@insekticid insekticid commented Jan 18, 2018

Q A
Branch? 2.8
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #25834
License MIT

@dunglas could you check it?

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jan 21, 2018

@insekticid would you mind adding a test case please?

Status: needs work

Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

When a test will be added.

@fabpot
Copy link
Member

fabpot commented Feb 7, 2018

@insekticid Can you work on adding a test case for your bug fix? That's something we need to avoid future regression. Or do you need help?

@nicolas-grekas
Copy link
Member

Status: needs work
(missing test case)

@insekticid
Copy link
Contributor Author

I need some time to go back to this issue and write some tests

@insekticid insekticid force-pushed the patch-1 branch 4 times, most recently from 05a9d10 to fb8fe4b Compare March 30, 2018 23:14
@insekticid
Copy link
Contributor Author

insekticid commented Mar 31, 2018

added tests

@dunglas ping

@Simperfit
Copy link
Contributor

Travis failure is unrelated.

Status: Needs Review

Copy link
Contributor

@Simperfit Simperfit left a comment

Choose a reason for hiding this comment

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

LGTM

@nicolas-grekas nicolas-grekas changed the title PropertyInfo\DoctrineExtractor - There is bug when indexBy is meta key [DoctrineBridge] Fix bug when indexBy is meta key Apr 14, 2018
@nicolas-grekas nicolas-grekas changed the title [DoctrineBridge] Fix bug when indexBy is meta key [DoctrineBridge] Fix bug when indexBy is meta key in PropertyInfo\DoctrineExtractor Apr 14, 2018
@fabpot
Copy link
Member

fabpot commented Apr 16, 2018

Isn't it something that also needs to be fixed in 2.8?

@nicolas-grekas nicolas-grekas changed the base branch from 3.4 to 2.8 April 20, 2018 09:36
@nicolas-grekas
Copy link
Member

Thank you @insekticid.

@nicolas-grekas nicolas-grekas merged commit 583759f into symfony:2.8 Apr 20, 2018
nicolas-grekas added a commit that referenced this pull request Apr 20, 2018
…rtyInfo\DoctrineExtractor (insekticid)

This PR was submitted for the 3.4 branch but it was merged into the 2.8 branch instead (closes #25841).

Discussion
----------

[DoctrineBridge] Fix bug when indexBy is meta key in PropertyInfo\DoctrineExtractor

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | #25834 <!-- #-prefixed issue number(s), if any -->
| License       | MIT

@dunglas could you check it?
<!--
- Bug fixes must be submitted against the lowest branch where they apply
  (lowest branches are regularly merged to upper ones so they get the fixes too).
- Features and deprecations must be submitted against the master branch.
- Replace this comment by a description of what your PR is solving.
-->

Commits
-------

583759f PropertyInfo\DoctrineExtractor - There is bug when indexBy is meta key
@nicolas-grekas nicolas-grekas modified the milestones: 3.4, 2.8 Apr 20, 2018
This was referenced Apr 30, 2018
@insekticid insekticid deleted the patch-1 branch March 1, 2021 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants