- Issue created by @miksha
- Merge request !6800Update Media.php in order to fix validation constraints overwrite. → (Open) created by miksha
- last update
over 1 year ago Composer error. Unable to continue. - 🇺🇸United States xjm
Thanks @miksha. The merge request needs to be created against 11.x (our main development branch); it will be backported to supported versions once committed to 11.x. In some cases the easiest thing to do is close the previous merge request and create a new one against 11.x.
- 🇷🇸Serbia miksha
- Status changed to Needs review
about 1 year ago 12:41am 9 September 2024 - Status changed to Needs work
about 1 year ago 2:12pm 9 September 2024 - 🇺🇸United States smustgrave
MR is 1244 commits behind.
Probably need a test case showing this issue.
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
larowlan → changed the visibility of the branch 3424200-media-overwrites-validation to hidden.
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
larowlan → changed the visibility of the branch 10.2.x to hidden.
- First commit to issue fork.
- 🇳🇿New Zealand danielveza Brisbane, AU
Addressed feedback. Adjusted the test to it captured the previous failing behaviour better.
Pipeline is green. Back to needs review.
- 🇬🇧United Kingdom oily Greater London
Added code review with recommendations. Not sure why PHPCS is not enforcing it but 'Implements hook_' docblocks seem to be always placed just above the hook attribute of the function (i checked by grepping 'Implements hook_').
- 🇳🇿New Zealand danielveza Brisbane, AU
I don't see any feedback comments on the PR, maybe you didn't publish the comments?
Regarding
'Implements hook_HOOK_NAME_WHATEVER_IT_MIGHT_BE' type docblocks seem to be always placed just above the hook attribute which itself is just above the function (i checked by grepping 'Implements hook_').
I assume you're talking about the hook placement with the
__invokefunction? If so, this is valid. See core.api.php which describes this scenario, and CronHook in the file module for an example of how this is already implemented in cores codebase :).Moving back to RTBC for now. Feel free to move back to NW if you have additional feedback on the PR outside of that
- 🇬🇧United Kingdom oily Greater London
Re #19 Thank you @danielveza yes I had forgotten to publish. Just did but my review is not relevant: having checked the links you provide I see you are correct.
