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.

Sign up for GitHub

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

Jump to bottom

Make the return-type of ClassReflection#getStartLine() explicit #161

Merged
merged 1 commit into from Nov 25, 2022
Merged

Make the return-type of ClassReflection#getStartLine() explicit #161

merged 1 commit into from Nov 25, 2022

Conversation

nicolas-grekas
Copy link
Contributor

Q A
Documentation no
Bugfix no
BC Break no
New Feature no
RFC no
QA no

This was removed in #154 but this phpdoc is useful to some static analyzers, so that they can know what the future return-type is going to be.

@nicolas-grekas
Make the return-type of ClassReflection::getStartLine() explicit
3fe1e32 
This was removed in laminas#154 but this phpdoc is useful to some static analyzers, so that they can know what the future return-type is going to be.

Signed-off-by: Nicolas Grekas <nicolas.grekas@gmail.com>
@Ocramius Ocramius added this to the 4.8.0 milestone Nov 25, 2022
@IonBazan
Copy link
Contributor

If we are aiming for ~8.1.0 || ~8.2.0 for v4.8.0, I think we don't really need to use #[ReturnTypeWillChange] and replace it with native union type there, right?

@Ocramius
Copy link
Member

@IonBazan changing a return type in a non-final class is still a BC break

@Ocramius Ocramius self-assigned this Nov 25, 2022
@Ocramius Ocramius merged commit 84532c3 into laminas:4.8.x Nov 25, 2022
@Ocramius Ocramius changed the title Make the return-type of ClassReflection::getStartLine() explicit Make the return-type of ClassReflection#getStartLine() explicit Nov 25, 2022
@IonBazan
Copy link
Contributor

Noted, let's do that for v5, if you don't mind

@nicolas-grekas nicolas-grekas deleted the patch-1 branch November 25, 2022 09:24
@Ocramius
Copy link
Member

@IonBazan I'd be really happy to make a lot of this stuff final for v5.

Once it's final, it is much much easier to maintain and change internals.

@nicolas-grekas
Copy link
Contributor Author

Thanks for merging quickly!

@Ocramius
Copy link
Member

No problem: just beware that I won't release until @IonBazan's #159 is also merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ocramius Ocramius Ocramius requested changes

Assignees

@Ocramius Ocramius

Labels
Documentation Enhancement
Projects
None yet
Milestone
4.8.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@nicolas-grekas @IonBazan @Ocramius