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

Upgrade to doctrine/coding-standard 10 #2481

Merged
merged 1 commit into from
Nov 18, 2022
Merged

Conversation

franmomu
Copy link
Contributor

@franmomu franmomu commented Oct 30, 2022

Q A
Type task
BC Break no
Fixed issues

Summary

It allows to use Doctrine CS 10 (based on doctrine/orm#10009)

@franmomu franmomu added the Task label Oct 30, 2022
@franmomu franmomu force-pushed the doctrine_cs branch 2 times, most recently from d22f7ce to 428dbb3 Compare October 30, 2022 20:56
@franmomu franmomu added this to the 2.4.3 milestone Oct 30, 2022
@franmomu franmomu requested a review from malarzm October 31, 2022 13:43
Copy link
Member

@malarzm malarzm left a comment

Choose a reason for hiding this comment

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

Let's hope this won't conflict much with #2473 👀

composer.json Outdated
@@ -40,7 +40,7 @@
},
"require-dev": {
"ext-bcmath": "*",
"doctrine/coding-standard": "^9.0",
"doctrine/coding-standard": "^9.0 || ^10.0",
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 requiring one or another?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

none 😬 I thought it was because the php version, but it's not necessary, thanks!

@@ -74,6 +74,7 @@ final class Query implements IteratorAggregate
public const TYPE_COUNT = 11;

public const HINT_REFRESH = 1;

Copy link
Member

Choose a reason for hiding this comment

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

This one's a pity but I guess there's nothing we can do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we can move the comment to the top, I'll give it a try.

/** @deprecated const was deprecated in doctrine/mongodb-odm 2.1 and will be removed in 3.0. Use Type::BOOL instead */
public const BOOLEAN = 'boolean';
public const INT = 'int';

Copy link
Member

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe grouping all deprecated constants?

Copy link
Member

Choose a reason for hiding this comment

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

That'd be an option as well :)

@franmomu
Copy link
Contributor Author

franmomu commented Oct 31, 2022

Let's hope this won't conflict much with #2473 👀

We can wait for that PR to get merged since these changes are automatic and do it in 2.5.x (I guess next release would be minor) and since we're requiring there phpunit/phpunit ^9.5 we won't need to exclude tests for the one line comment.

@franmomu franmomu force-pushed the doctrine_cs branch 2 times, most recently from 9dd1064 to 8d1bfb1 Compare November 7, 2022 17:50
@franmomu franmomu changed the base branch from 2.4.x to 2.5.x November 7, 2022 17:51
@franmomu franmomu closed this Nov 7, 2022
@franmomu franmomu reopened this Nov 7, 2022
@franmomu franmomu modified the milestones: 2.4.3, 2.5.0 Nov 7, 2022
@IonBazan IonBazan merged commit 52d4a22 into doctrine:2.5.x Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants