-
Notifications
You must be signed in to change notification settings - Fork 82
[v5] Adjust code samples #2748
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
[v5] Adjust code samples #2748
Conversation
$context = $payment->getContext(); | ||
|
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.
$context
seems not used between this set and its override at line 97 (below // Create a new payment
)
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.
You're right, it doesn't make a lot of sense in the context of the Command - I've included it to improve the doc a bit:
Please compare:
https://ez-systems-developer-documentation--2748.com.readthedocs.build/en/2748/commerce/payment/extend_payment/#attach-custom-data-to-payments
and
https://doc.ibexa.co/en/latest/commerce/payment/extend_payment/#attach-custom-data-to-payments
I could add the getContext
line somewhere else and then "glue" 2 includes together, I just prefered having it in one include as I think it's easier to maintain
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.
That's tricky!
I didn't see that extend_payment.md
uses it in reverse order…
Maybe a comment in the cut-out line to explain why it's there?
$context = $payment->getContext(); | |
$context = $payment->getContext(); | |
// Will be overridden later but used to illustrate `getContext()` |
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.
Added in 79c5c1a - I'll wait for the "check code samples" job to make sure I did it right
...ck_office/search/src/Search/Serializer/Normalizer/Suggestion/ProductSuggestionNormalizer.php
Show resolved
Hide resolved
code_samples/ change reportReport's diff is too long to be displayed in a comment. |
Target: 5.0 only!
See the failing build on v5:
https://github.com/ibexa/documentation-developer/actions/runs/15046924482/job/42291716387?pr=2739
There are two types of errors:
Commands
I've switched to registering the Commands using the
AsCommand
attribute: https://symfony.com/doc/current/console.html#registering-the-commandNormalizers and denormalizers
The interfaces have changed, see:
getSupportedTypes
to allow better performance symfony/symfony-docs#18042