Closed (duplicate)
Project:
Upgrade Status
Version:
4.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
25 Jan 2022 at 16:55 UTC
Updated:
20 Feb 2026 at 15:08 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
tmaiochi commentedWorking on it.
Comment #3
tmaiochi commentedI fixed all dependency injection errors in PHPCS. There are other PHPCS errors and warnings that will be fixed in other issues.
Comment #4
victoria-marina commentedI'll review it.
Comment #5
victoria-marina commented@tmaiochi as the class UpgradeStatusCommands is a service, it needs to have the new injected dependency as an argument on the .services file.
Comment #6
tmaiochi commented@victoria-marina Thanks, I forgot to change there. This is a new patch with this change.
Comment #7
lucienchalom commentedHello, I reviewed and found that on file UpgradeStatusForm.php, because of the injection of the dependency of users Roles, the use statement of Roles is no longer used, so we have a new phpcs error for unused use statement.
Besides that, everything looks good and the module is working as far as I can test it.
Comment #8
tmaiochi commentedThanks for your review @lucienchalom, I removed this unused use statement in this new patch.
Comment #9
tmaiochi commentedComment #10
lucienchalom commentedThank you!
With that patch, the dependency injections were fixed, no new errors were added, everything looks good and the module is working as far as I can test it.
Moving to RTBC
Comment #11
gábor hojtsyThanks, this looks good other than these two IMHO:
I would say this is "The key-value storage" or somesuch. What is the comment for such properties elsewhere in this module or core?
Is this the current user's permissions? That would be clearer.
Also as the drush command does not have automated test coverage, did you manually test that it still works?
Comment #12
tmaiochi commentedHey @Gábor Hojtsy, I changed the documentation for these ones. About the drush command I tested manually and work fine.
Comment #13
lucienchalom commented"The key value." was changed to "Defines the key/value store factory interface."
which I agree is better explained.
For some reason, the User permission is not showing on the interdiff? but was changed to "Defines an interface to list available permissions."
I tested in the interface, but now I also tested the dush command for checkstyle.
Thank you for the reminder!
Comment #14
gábor hojtsySorry for not integrating this when it was done. Unfortunately definitely does not apply anymore. Also 3.x is not supported anymore.
Drush commans now have some automated test coverage so that will not need to be manually tested.
Comment #15
benstallings commentedComment #17
gábor hojtsyI did add all credits to #3459216: Fix all phpcs issues though, so your efforts are not uncredited. Thanks all!