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

Remove baseline #473

Merged
merged 1 commit into from
Dec 5, 2024
Merged

Remove baseline #473

merged 1 commit into from
Dec 5, 2024

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Nov 29, 2024

This implies bumping psr/log to v2, which adds type declarations to its
interface.

@greg0ire greg0ire marked this pull request as draft November 29, 2024 18:35
@greg0ire
Copy link
Member Author

Requires #475 , a merge up and a bump of the psr/log dependency.

@greg0ire greg0ire changed the title Reduce baseline Remove baseline Nov 30, 2024
@greg0ire greg0ire force-pushed the reduce-baseline branch 2 times, most recently from 5a0b9f5 to fe0a2a2 Compare November 30, 2024 09:09
@@ -9,6 +9,7 @@

class RequiredConstructorArgsFixtures implements ORMFixtureInterface
{
/** @phpstan-ignore constructor.unusedParameter */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only PHPStan issue left in the project, I propose we switch to phpstan-ignore for it, and now that PHPStan has error identifiers, maybe we should do that for everything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for the constructor to have an argument for the test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That class was introduced in ee5eb61, which has a commit message that explains the reason.

    Also fixing a bug where getDependencies()
    
    Basically, currently getDependencies() won't work if any of your classes
    have required constructor args. We've added a clear error message for
    this situation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just for a test but maybe the ignore isn't needed when the property is set to public.

@greg0ire greg0ire force-pushed the reduce-baseline branch 2 times, most recently from 67f01c4 to 5b8dd5e Compare November 30, 2024 09:20
@greg0ire greg0ire marked this pull request as ready for review November 30, 2024 09:21
@@ -9,6 +9,7 @@

class RequiredConstructorArgsFixtures implements ORMFixtureInterface
{
/** @phpstan-ignore constructor.unusedParameter */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just for a test but maybe the ignore isn't needed when the property is set to public.

This implies bumping psr/log to v2, which adds type declarations to its
interface.
@greg0ire greg0ire merged commit ebfb3a5 into doctrine:4.0.x Dec 5, 2024
12 checks passed
@greg0ire greg0ire deleted the reduce-baseline branch December 5, 2024 06:45
@greg0ire greg0ire added this to the 4.0.0 milestone Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants