Problem/Motivation

Module don't support Drupal 11.x version.

Command icon 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

krystianbrzoza created an issue. See original summary.

ericgsmith’s picture

Status: Active » Needs work

@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.

krystianbrzoza’s picture

Status: Needs work » Needs review

@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.

ericgsmith’s picture

Version: 2.0.0 » 2.x-dev
ericgsmith’s picture

@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.

ericgsmith’s picture

Status: Needs review » Needs work

Updating 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)

krystianbrzoza’s picture

Judging 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.

ericgsmith’s picture

Thanks @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.

krystianbrzoza’s picture

Status: Needs work » Needs review

@ericgsmith
As promised, I 've added bug fixes and excluded one false-positive error that often appears in Drupal.

Feel free to test it :)

ericgsmith’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @krystianbrzoza - can confirm this is still working as expected. Thank you

rosk0’s picture

Status: Reviewed & tested by the community » Fixed

Thanks 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.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.