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

Convert proxy factory auto generate mode to integer #815

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@dragosprotung
Copy link
Contributor

dragosprotung commented Aug 30, 2017

No description provided.

@dragosprotung dragosprotung force-pushed the dragosprotung:2.8 branch from 67300ec to b11a7f9 Aug 30, 2017

@lcobucci
Copy link
Member

lcobucci left a comment

@dragosprotung some small nitpickings and CS fixes to be done (I'm sending a PR that will add PHPCS to help to detect and use phpcbf to fix them).

And please use master as the destination branch, we'll handle the necessary backporting 😄

Thanks for you contribution 👍

*
* @return self
*/
public static function invalidAutoGenerateMode($value)

This comment has been minimized.

@lcobucci

lcobucci Aug 30, 2017

Member

Please add the return type declation

[$proxyGenerator, $metadataFactory, $autoGenerate]
);
$this->assertAttributeSame($expected, 'autoGenerate', $proxyFactory);

This comment has been minimized.

@lcobucci

lcobucci Aug 30, 2017

Member

Please use self::assert*()

* @param mixed $autoGenerate
* @param int $expected
*/
public function testNoExceptionIsThrownForValidIntegerAutoGenerateValues($autoGenerate, $expected)

This comment has been minimized.

@lcobucci

lcobucci Aug 30, 2017

Member

Please add the type declaration on $expected and : void as return type

@dragosprotung dragosprotung force-pushed the dragosprotung:2.8 branch from b11a7f9 to d9f7f4a Aug 30, 2017

@dragosprotung dragosprotung changed the base branch from 2.8 to master Aug 30, 2017

@dragosprotung

This comment has been minimized.

Copy link
Contributor Author

dragosprotung commented Aug 30, 2017

I changed the commit base to master and resolved in GitHub UI the conflict and this made a bit of a mess. Don't know how to have a cleaner PR. Shall I close this one and submit a new one ?

@lcobucci

This comment has been minimized.

Copy link
Member

lcobucci commented Aug 30, 2017

@dragosprotung we usually use git rebase to sync branches and solve conflicts. It's up to you to decide which path you want to go. Personally I'd advise to give it a try, you might learn about some nice git features 😉

@dragosprotung

This comment has been minimized.

Copy link
Contributor Author

dragosprotung commented Aug 30, 2017

Usually I use rebase, but not this time.
Submitted #816

@Ocramius Ocramius added the Bug label Aug 31, 2017

@Ocramius Ocramius added this to the 2.8.1 milestone Aug 31, 2017

Ocramius added a commit that referenced this pull request Aug 31, 2017

Merge branch 'fix/#815-#816-revert-bc-break-preventing-non-boolean-pr…
…oxy-generator-modes-2.8' into 2.8

Backport #815
Backport #816

Ocramius added a commit that referenced this pull request Aug 31, 2017

@dragosprotung dragosprotung deleted the dragosprotung:2.8 branch Aug 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.