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

fix static constructor support #301

Merged
merged 2 commits into from
Dec 28, 2015

Conversation

bendavies
Copy link
Contributor

There is a limitation/bug in the support for static constructors, in that the static constructor required arguments needed to match the __construct method.

This PR fixes that use case.

@bendavies bendavies force-pushed the static-constructors branch 2 times, most recently from 5b1f62f to 952eb59 Compare December 23, 2015 09:54
@bendavies bendavies changed the title extend static constructor support fix static constructor support Dec 23, 2015
@bendavies
Copy link
Contributor Author

@Seldaek any chance of a quick review Jordi?

@tshelburne
Copy link
Collaborator

@bendavies Makes sense to me - two things:

  1. Why did you remove the arguments we had in the User constructor? It doesn't seem related to this PR.
  2. Please create a test for your use-case that fails without your fix so we can ensure it doesn't regress in the future.

Thanks for this!

@bendavies
Copy link
Contributor Author

@tshelburne I've made this into two commits to make it a bit more obvious.

  1. I've changed it up a bit. There is now a new model just for static constructor testing.
  2. It's already covered. After the first commit, testLoadCallsStaticConstructorIfProvided will fail. The 2nd commit fixes it.

Thanks.

tshelburne added a commit that referenced this pull request Dec 28, 2015
@tshelburne tshelburne merged commit c05812b into nelmio:master Dec 28, 2015
@tshelburne
Copy link
Collaborator

@bendavies Thanks for this!

@bendavies
Copy link
Contributor Author

@tshelburne no problem. thanks for the merge!

@bendavies
Copy link
Contributor Author

@tshelburne any chance of a tag?

@tshelburne
Copy link
Collaborator

@bendavies Done. Thanks again.

@bendavies
Copy link
Contributor Author

Cheers

On Mon, 4 Jan 2016, 18:49 Tim Shelburne [email protected] wrote:

@bendavies https://github.com/bendavies Done. Thanks again.


Reply to this email directly or view it on GitHub
#301 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants