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

$hydrationMode throughout can be a string as well as int (for custom modes) #7441

Merged
merged 1 commit into from
Nov 20, 2018

Conversation

asgrim
Copy link
Contributor

@asgrim asgrim commented Oct 24, 2018

As per docs: https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/reference/dql-doctrine-query-language.html#custom-hydration-modes

You can add custom hydration modes. Whilst running phpstan on a project, it discovered that whilst addCustomHydrationMode accepts a string, when you call a method like getResult the $hydrationMode parameter only accepts int and therefore phpstan complains.

I've added string| to all existing type hints.

Ocramius
Ocramius previously approved these changes Oct 25, 2018
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Ocramius Ocramius added this to the 2.6.3 milestone Oct 25, 2018
@Majkl578
Copy link
Contributor

This should target 2.6 branch.
Also there is more int typehints that have to be changed (at least AbstractQuery::$_hydrationMode, getter and setter).

@Majkl578
Copy link
Contributor

Ah nevermind, didn't read the diff properly. :|
Still needs to be retargeted to 2.6 though.

@Majkl578 Majkl578 changed the base branch from master to 2.6 November 10, 2018 19:58
@Majkl578
Copy link
Contributor

Base branch changed, but rebase is required - @asgrim do you want to do it (and keep GPG sig) or want me to handle it?

@asgrim
Copy link
Contributor Author

asgrim commented Nov 12, 2018

I'm on it 👍

@asgrim
Copy link
Contributor Author

asgrim commented Nov 12, 2018

Rebased onto 2.6 and use string|int consistently instead of int|string 👍

@lcobucci
Copy link
Member

This LGTM just a tiny question: do we have a test using a custom hydration mode using strings? I'm on mobile now so can't check at the moment. It's also fine to move this to another issue/pr

@lcobucci lcobucci self-requested a review November 20, 2018 08:14
@lcobucci
Copy link
Member

This LGTM just a tiny question: do we have a test using a custom hydration mode using strings? I'm on mobile now so can't check at the moment. It's also fine to move this to another issue/pr

We do have tests 👍

@lcobucci lcobucci merged commit 728e6e1 into doctrine:2.6 Nov 20, 2018
@lcobucci lcobucci assigned lcobucci and unassigned asgrim Jan 16, 2019
ixarlie added a commit to surexdirect/doctrine2 that referenced this pull request Jul 10, 2019
v2.6.3

[![Build Status](https://travis-ci.org/doctrine/doctrine2.svg?branch=v2.6.3)](https://travis-ci.org/doctrine/doctrine2)

This release provides fixes for many things, specially:

- Regression in commit order calculation
- BC-break in `EntityManager#find()` using optimistic lock outside of
  transaction
- PHP 7.3 compatibility issues

--------------------------------------------

- Total issues resolved: **8**
- Total pull requests resolved: **26**
- Total contributors: **26**

Documentation
-------------

 - [7472: fix incorrect phpdoc typehint](doctrine#7472) thanks to @seferov
 - [7465: Fixes tiny typo in the 'Working with DateTime instances' documentation](doctrine#7465) thanks to @unguul
 - [7444: Fixed URLs of doctrine-mapping.xsd in docs](doctrine#7444) thanks to @Naitsirch
 - [7441: $hydrationMode throughout can be a string as well as int (for custom modes)](doctrine#7441) thanks to @asgrim
 - [7435: Fix a typo on Documentation](doctrine#7435) thanks to @oguzdumanoglu
 - [7434: Removed FAQ paragraph stating public variables are disallowed](doctrine#7434) thanks to @Naitsirch and @flaushi
 - [7423: Update association-mapping.rst](doctrine#7423) thanks to @ThomasLandauer
 - [7421: JIRA to Github issues on Limitations and Known Issues](doctrine#7421) thanks to @seferov
 - [7412: Some formatting improvements](doctrine#7412) thanks to @ThomasLandauer
 - [7411: Autoload error when following the Getting Started Guide](doctrine#7411) thanks to @ThomasLandauer
 - [7401: &doctrine#91;docs&doctrine#93; Fix docblock in `inheritance-mapping.rst`](doctrine#7401) thanks to @bobdenotter
 - [7397: Update getting-started.rst](doctrine#7397) thanks to @eibt
 - [7394: Class 'Doctrine\Common\Persistence\Mapping\Driver\AnnotationDriver' not found](doctrine#7394) thanks to @ekosynth
 - [7378: Typo fix](doctrine#7378) thanks to @BenMorel
 - [7377: Fix query andX doctype](doctrine#7377) thanks to @sserbin
 - [7374: Deprecation message in documentation for YAML](doctrine#7374) thanks to @SenseException and @iltar
 - [7360: Document getPartialReference() properly](doctrine#7360) thanks to @lcobucci

Bug
---

 - [7471: Fix parameter value processing for objects with unloaded metadata](doctrine#7471) thanks to @alcaeus
 - [7367: Fix for BC break in 2.6.2 when calling EM::find() with LockMode::OPTIMISTIC outside of a TX](doctrine#7367) thanks to @timdev
 - [7328: Handle removed parameters by tree walker in Paginator](doctrine#7328) thanks to @plfort
 - [7325: Make code php 7.3 lint-compatible](doctrine#7325) thanks to @paxal
 - [7317: &doctrine#91;XML&doctrine#93; Fix default value of many-to-many order-by to ASC](doctrine#7317) thanks to @alexdenvir
 - [7260: Fix the handling of circular references in the commit order calculator](doctrine#7260) thanks to @stof
 - [6830: fix applying column options on foreign key columns](doctrine#6830) thanks to @Tobion

Improvement
-----------

 - [7428: CI: Test against PHP 7.3](doctrine#7428) thanks to @Majkl578
 - [7363: Fix compatibility with phan](doctrine#7363) thanks to @philippe-unitiz
 - [7345: Correct DOMDocument constructor in test](doctrine#7345) thanks to @guilliamxavier
 - [7307: Fix remaining usages of deprecated ClassLoader and Inflector from doctrine/common](doctrine#7307) thanks to @Majkl578 and @simonwelsh
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.

5 participants