Assignment not done by reference #792

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

superdweebie commented Jan 30, 2014

From the constructor arguments, clearly this assignment is meant to be by ref.

Owner

jwage commented Jan 30, 2014

Hi @superdweebie can we add a test case to cover this usage?

Owner

jmikola commented Jan 30, 2014

Do we have an examples of userland code using PreLoadEventArgs? Maybe in https://github.com/l3pp4rd/DoctrineExtensions?

Contributor

superdweebie commented Jan 30, 2014

@jwage I'll add a test case.

@jmikola I'm in userland, and I was using it last night :)

Contributor

superdweebie commented Feb 3, 2014

@jwage test added.
Also added an entry to gitignore for netbeans users. Hope that's ok.

Owner

jmikola commented Feb 3, 2014

@superdweebie: It looks like the .idea ignore came from 66780d1 as part of #401. Really, we shouldn't be ignoring any IDE-specific files in a project .gitignore. That's best done globally.

Contributor

superdweebie commented Feb 3, 2014

@jmikola thanks for the git tip. I'll remove it.

Contributor

superdweebie commented Feb 3, 2014

@jmikola done

Owner

jmikola commented Feb 3, 2014

@superdweebie: Can you squash the last two commits (e.g. git rebase -i HEAD~2, mark the second as a fixup, and git push -f origin <branch>)? No need to have it added and removed separately :)

Contributor

superdweebie commented Feb 3, 2014

@jmikola cleanup done.

Owner

jwage commented Feb 4, 2014

It looks like that test is PHP 5.4 only?

Contributor

superdweebie commented Feb 4, 2014

@jwage you're right. Sorry. I haven't used 5.3 for a long time.

Owner

jmikola commented Feb 4, 2014

@superdweebie: Manually merged in 85f6898. Thanks.

jmikola closed this Feb 4, 2014

@jmikola jmikola added a commit that referenced this pull request Feb 5, 2014

@jmikola jmikola Fix PreLoadEventArgsTest for PHP 5.3
Originally introduced in commit 13338b2 for #792.
98a9fe0

@jmikola jmikola commented on the diff Feb 5, 2014

...ine/ODM/MongoDB/Tests/Events/PreLoadEventArgsTest.php
+namespace Doctrine\ODM\MongoDB\Tests\Events;
+
+use Doctrine\ODM\MongoDB\Event\PreLoadEventArgs;
+use Documents\Group;
+
+class PreLoadEventArgsTest extends \Doctrine\ODM\MongoDB\Tests\BaseTest
+{
+ public function testGetData()
+ {
+ $document = new Group('test');
+ $dm = $this->dm;
+ $data = array('id' => '1234', 'name' => 'test');
+
+ $eventArgs = new PreLoadEventArgs($document, $dm, $data);
+
+ $this->assertEquals('test', $eventArgs->getData()['name']);
@jmikola

jmikola Feb 5, 2014

Owner

Using [] on a return value was still problematic for 5.3. Fixed in #794.

@jmikola jmikola added a commit that referenced this pull request Feb 5, 2014

@jmikola jmikola Fix PreLoadEventArgsTest for PHP 5.3
Originally introduced in commit 13338b2 for #792.
9308f4a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment