-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
DevKit updates for 4.x branch #8133
Conversation
I think the SF 7.0 compat is still missing a few changes. |
/** | ||
* @internal | ||
* | ||
* @phpstan-ignore-next-line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error can't be ignored like this.
Build was green on #8127 (?) |
Maybe between the RC and the stable version, something changed? |
af8ca31
to
ee88214
Compare
The check |
1325bec
to
701ab92
Compare
/** @internal */ | ||
interface CompatibleValueResolverInterface extends ValueResolverInterface | ||
interface CompatibleValueResolverInterface extends ArgumentValueResolverInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VincentLanglet isn't it the other way around?
this code gives me deprecations right now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you try
if (interface_exists(ArgumentValueResolverInterface::class) && !interface_exists(ValueResolverInterface::class)) {
/** @internal */
interface CompatibleValueResolverInterface extends ArgumentValueResolverInterface
{
}
} else {
/** @internal */
interface CompatibleValueResolverInterface extends ValueResolverInterface
{
}
}
?
If it works for you PR is welcome
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Question is more about what to do between the versions:
- 3.1.0 - 6.2.0 there was only ArgumentValueResolverInterface
- 6.2.0 - 7.0.0 there are both ArgumentValueResolverInterface and ValueResolverInterface, the first one is deprecated.
- 7.0.0 - there is only ValueResolverInterface
what would be the right choice for 6.2.0 - 7.0.0 ? Which Interface should we extend?
there shouldn't be extra logic needed, its just ArgumentValueResolverInterface =>ValueResolverInterface
as it was before, using ValueResolverInterface if able was better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to open a pr with what you have in mind and try having a green build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should work:
#8139
No description provided.