Problem/Motivation
Module don't support Drupal 11.x version.
Issue fork permission_watchdog-3499655
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
ericgsmith commented@Krystian Brzoza thank you for the work on this! This wasn't set to ready for review but as its been a long time I'm assuming you are not working on this anymore.
I've added some comments to the MR - I can pick this up unless you want to continue working on it.
There is existing test coverage for this module so it would also be good to show that working in CI.
Comment #4
krystianbrzoza commented@ericgsmith
Oh, I completely forgot about that issue.
I added a few fixes to the branch.
I think it should now be fully functional for Drupal 10 and 11.
Comment #5
ericgsmith commentedComment #6
ericgsmith commented@krystianbrzoza I was just about to comment that I think this is good to go but I see you are still working on it - introducing oop hooks etc. Just be mindful of expanding the scope beyond what we need here - I'll jump out again but can review when you are done.
Comment #7
ericgsmith commentedUpdating issue status to reflect latest pipeline failure - IMO we can drop that last commit and open up follow ups for oop hook conversion and if needed the tightening of phpstan rules, but I think up to maintainers if they want project specific phpstan config over the gitlab ci defaults. If we do that then I think the last green pipeline is RTCB (although I made some changes there so would need somebody else to review those)
Comment #8
krystianbrzoza commentedJudging by the maintainers' activity, it seems that the module has been abandoned.
As for the additional changes with phpstan and hooks, I think it will be a nice addition to the release (the extra work has already been done), and the more demanding rules will allow for the creation of higher quality code.
I will try to add a commit in the near future that will fix the errors returned by phpstan. Unless you manage to fix those few errors.
Comment #9
ericgsmith commentedThanks @krystianbrzoza - yes sorry I didn't mean to sound discouraging - yes the work has been done and it is good work. I just meant in the context of a maintainer needing to review this, keeping the issue focused and limited in scope may have be easier to review. I'm not talking about not doing the work, but splitting it across issues to keep the reviews smaller.
Anyway - as you say the work is done - if you are able to fix or add to the baseline those last phpstan issues I can review - @rosk0 became maintainer to get the v2 / d9 release done years ago - I can ping him once we have everything ready here.
Comment #10
krystianbrzoza commented@ericgsmith
As promised, I 've added bug fixes and excluded one false-positive error that often appears in Drupal.
Feel free to test it :)
Comment #11
ericgsmith commentedThanks @krystianbrzoza - can confirm this is still working as expected. Thank you
Comment #13
rosk0Thanks for the work Team!
This has been merged and release in https://www.drupal.org/project/permission_watchdog/releases/2.1.0.
Krystian Brzoza, great job done there in modernizing the code base, but next time ( on any other project ), please keep the changes scoped to a title of the issue/merge/pull request - it is easier to review, less cognitive load, and more chances that your work would be accepted.