- Status changed to Needs review
about 1 year ago 6:35am 9 December 2023 - π³πΏNew Zealand quietone
It was simple task to correct the @see. The harder part was generating the proxy files. I happened to test with the book module and the script was not providing any feedback. I eventually found π Provide a nice error message when trying to run the generate proxy class script on uninstalled/missing modules Active which informed me that the book module needs to be enabled. With that fixed I then went to generate the files. I didn't see an option to generate all the files so I wrote my own script to do that.
Looking at the changed proxy files they all have the expected change except for ./core/modules/field/src/ProxyClass/FieldUninstallValidator.php. That has a method added that was added to ./core/modules/field/src/FieldUninstallValidator.php in #2392815: Module uninstall validators are not used to validate a config import β . I suspect that is out of scope for this issue.
- Status changed to Needs work
about 1 year ago 12:31am 10 December 2023 - πΊπΈUnited States smustgrave
Could the issue summary be updated? Not super clear what's being updated.
Also will this require a test case?
- Status changed to Needs review
about 1 year ago 3:36am 10 December 2023 - π³πΏNew Zealand quietone
This is a fix to documentation, specifically @see statements and we don't test documentation changes. As for generating the proxy classes, that is done via a script, so again no tests needed.
- πΊπΈUnited States smustgrave
What about the change to core/modules/field/src/ProxyClass/FieldUninstallValidator.php
- π³πΏNew Zealand quietone
@smustgrave, as the reviewer your opinion is valuable. What do you think should happen?
- πΊπΈUnited States smustgrave
More asking. This is a documentation change but the one file is adding a new method
- π³π±Netherlands spokje
Dropping my 0.02 EUR...
Changes in phpdoc are fine/excellent.
The new method that was missed in #2392815: Module uninstall validators are not used to validate a config import β should be added, but the fact that it was missed and that @quietone needed to make a script to recreate all proxy classes shows IMHO that we're (at least) missing test-coverage.For that reason I would say, it needs a follow-up to add it and make sure we have "something" that makes sure that this doesn't happen in the future. Especially the latter justifying the admin-overhead of a follow-up
- πΊπΈUnited States smustgrave
Thanks @Spokje! @quietone are you in agreement?
- Status changed to Needs work
about 1 year ago 2:41pm 21 December 2023 - π·πΊRussia zniki.ru
@quietone can you please share the script to generate all the files?