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 MethodGenerator parameter sorting, when an explicit sorting is provided #90

Merged

Conversation

willjones9
Copy link

The MethodGenerator doesn't seem to take any notice of the ParameterGenerator position property. This causes issues when adding properties out of order, especially with optional and variadic parameters.

Will Jones added 3 commits July 7, 2021 23:49
The MethodGenerator doesn't seem to take any notice of the ParameterGenerator position property. This causes issues when adding properties out of order, especially with optional and variadic parameters.

Signed-off-by: Will Jones <[email protected]>
Signed-off-by: Will Jones <[email protected]>
Signed-off-by: Will Jones <[email protected]>
src/Generator/MethodGenerator.php Outdated Show resolved Hide resolved
Will Jones and others added 2 commits July 8, 2021 09:27
Signed-off-by: Will Jones <[email protected]>
@willjones9 willjones9 force-pushed the bugfix-MethodGenerator-Parameter-Position branch from e5905df to 66a3464 Compare July 8, 2021 16:13
src/Generator/MethodGenerator.php Outdated Show resolved Hide resolved
test/Generator/MethodGeneratorTest.php Outdated Show resolved Hide resolved
test/Generator/MethodGeneratorTest.php Outdated Show resolved Hide resolved
test/Generator/MethodGeneratorTest.php Show resolved Hide resolved
@Ocramius
Copy link
Member

Ocramius commented Jul 8, 2021

@willjones-stratagem I just noticed that this patch targets 4.5.x, but this is a bugfix.

Should we change the target branch to 4.4.x?

Update tests, new method visibility, update psalm-baseline

Signed-off-by: Will Jones <[email protected]>
@willjones9
Copy link
Author

@Ocramius retargeting it works for me

@willjones9 willjones9 changed the base branch from 4.5.x to 4.4.x July 8, 2021 16:56
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 the Bug Something isn't working label Jul 9, 2021
@Ocramius Ocramius self-assigned this Jul 9, 2021
@Ocramius Ocramius added this to the 4.4.1 milestone Jul 9, 2021
@Ocramius Ocramius changed the title Fix MethodGenerator Parameter Position Fix MethodGenerator parameter sorting, when an explicit sorting is provided Jul 9, 2021
@Ocramius Ocramius merged commit 0919a1b into laminas:4.4.x Jul 9, 2021
@Ocramius
Copy link
Member

Ocramius commented Jul 9, 2021

Thanks @willjones-stratagem!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants