Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[FrameworkBundle][Validator] Fix deprecations from Doctrine Annotations+Cache#41230

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@derrabus
Copy link
Member

@derrabusderrabus commentedMay 14, 2021•
edited
Loading

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsN/A
LicenseMIT
Doc PRN/A
  • Doctrine Annotations'CachedReader is deprecated. Let's not use it if we don't have to.
  • Doctrine Cache 2 has been released. Since we're mostly only using the interfaces, we can indicate compatibility.

Paslm is going to complain about missing classes, which is kind-of expected here. 🙂

chalasr reacted with thumbs up emoji
@carsonbotcarsonbot added this to the4.4 milestoneMay 14, 2021
@carsonbotcarsonbot changed the titleFix deprecations from Doctrine Annotations+Cache[FrameworkBundle][Validator] Fix deprecations from Doctrine Annotations+CacheMay 14, 2021
@derrabusderrabusforce-pushed thebugfix/annotations-cache-deprecations branch from12721f8 toaae8b2dCompareMay 14, 2021 20:07
@derrabus
Copy link
MemberAuthor

cc@alcaeus

@derrabus
Copy link
MemberAuthor

The Travis failure is particularly interesting. In debug mode,PsrCachedReader seems to behave differently thanCachedReader does. 👀

@carsonbot

This comment has been minimized.

Copy link
Contributor

@alcaeusalcaeus left a comment

Choose a reason for hiding this comment

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

I really appreciate the fact that you managed to introduce compatibility with a new major release in a patch release with a nice downgrade to an uncached reader if everything else fails. Defensive programming to perfection! 👏

derrabus reacted with heart emoji
@ostrolucky
Copy link
Contributor

He didn't manage that yet, though. He mentioned he has problems that tests discovered new cache having different behaviour.

@derrabusderrabusforce-pushed thebugfix/annotations-cache-deprecations branch fromaae8b2d to2c9dd17CompareMay 16, 2021 10:38
@derrabusderrabusforce-pushed thebugfix/annotations-cache-deprecations branch from2c9dd17 to3b8dfc7CompareMay 16, 2021 11:41
@alcaeus
Copy link
Contributor

He mentioned he has problems that tests discovered new cache having different behaviour.

I'll take the blame for that, since the difference in behaviour comes from doctrine/annotations providing the wrong functionality. They're supposed to have the same behaviour with different cache libraries.

@derrabus
Copy link
MemberAuthor

derrabus commentedMay 16, 2021•
edited
Loading

The issueshould be has been fixed bydoctrine/annotations#412.

@derrabusderrabusforce-pushed thebugfix/annotations-cache-deprecations branch 2 times, most recently from99bc0bd to1cfd50cCompareMay 16, 2021 18:23
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

(but let's release annotations 1.13.1 before merging?)

@derrabusderrabusforce-pushed thebugfix/annotations-cache-deprecations branch from1cfd50c to4c12a26CompareMay 16, 2021 18:41
@derrabusderrabusforce-pushed thebugfix/annotations-cache-deprecations branch from4c12a26 toaa4b8bdCompareMay 16, 2021 18:47
@alcaeus
Copy link
Contributor

doctrine/annotations 1.13.1 is released, so feel free to bump 👍

@derrabus
Copy link
MemberAuthor

(but let's release annotations 1.13.1 before merging?)

1.13.1 has been tagged.

@derrabusderrabusforce-pushed thebugfix/annotations-cache-deprecations branch fromaa4b8bd toec51c21CompareMay 16, 2021 21:41
Copy link
Member

@NyholmNyholm left a comment

Choose a reason for hiding this comment

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

Thank you. Just a minor question to clarify

@derrabusderrabus merged commit373528f intosymfony:4.4May 17, 2021
@derrabusderrabus deleted the bugfix/annotations-cache-deprecations branchMay 17, 2021 19:36
This was referencedMay 19, 2021
fabpot added a commit that referenced this pull requestMay 31, 2021
This PR was merged into the 5.3 branch.Discussion----------[FrameworkBundle] Remove redundant cache service| Q             | A| ------------- | ---| Branch?       | 5.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | N/A| License       | MIT| Doc PR        | N/AFollows#40444 and#41230.`@Nyholm` and I have fixed the same problem on different branches. Merging both efforts, we've created two cache services that are supposed to serve the same purpose. This PR suggests to remove one of them.Commits-------3b197fe [Framework] Remove redundant cache service
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@stofstofstof approved these changes

@NyholmNyholmNyholm approved these changes

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

+1 more reviewer

@alcaeusalcaeusalcaeus approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

7 participants

@derrabus@carsonbot@ostrolucky@alcaeus@nicolas-grekas@stof@Nyholm
[8]ページ先頭
©2009-2025 Movatter.jp