Followup from #2328411: Convert all permissions to yml files and permission callbacks for the actual removal of hook_permission().

CommentFileSizeAuthor
#7 remove-hook_permission-2338475-7.patch9.28 KBherom
#7 interdiff-2338475-5-7.txt601 bytesherom
#5 remove-hook_permission-2338475-5.patch9.27 KBherom

Comments

geerlingguy’s picture

geerlingguy commented

Are we going to remove entirely, or just mark as deprecated for the time being? This change will likely break 90+% of the modules currently ported to Drupal 8, so it might be best to give them time to move to YAML before a later beta or maybe even a minor version.

herom’s picture

herom commented

My personal opinion is to remove it entirely. A lot of modules are going to break already, from the FormState changes, the Controller -> Handler renames, or a lot of other changes since alpha14. I'd rather we break the modules for people chasing alphas than those chasing betas.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Yeah if permissions callbacks is the new way than keeping the hook is probably pointless.

dawehner’s picture

dawehner
Primary language German
commented

Totally but yeah we use the typical workflow of converting everything, marking the origin as deprecated and then remove the origin.

herom’s picture

herom commented
Status: Active » Needs review
StatusFileSize
new remove-hook_permission-2338475-5.patch9.27 KB

The parent issue got in.
Here's a patch based on dawehner's changes in 2328411#49 that completely removed hook_permission() and all of its mentions.

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/modules/user/src/Access/PermissionAccessCheck.php
@@ -13,7 +13,7 @@
- * Determines access to routes based on permissions defined via permissions.yml.
+ * Determines access to routes based on permissions defined via $module.permissions.yml files.

>80 chars :(

In general this looks nice!

herom’s picture

herom commented
StatusFileSize
new interdiff-2338475-5-7.txt601 bytes
new remove-hook_permission-2338475-7.patch9.28 KB

fixed #6.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Well, yeah these are really details here.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

The CR https://www.drupal.org/node/2311427 needs updating.

Committed e1081c6 and pushed to 8.0.x. Thanks!

  • alexpott committed e1081c6 on 8.0.x 
    Issue #2338475 by herom: Remove hook_permission().

Status: Fixed » Closed (fixed)

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