All methods have docblocks stating

 /**
   * Implements EntityAccessControllerInterface::XXX().
   */

should be

 /**
   * Implements EntityAccessControllerInterface::XXXAccess().
   */

in line with the interface

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan’s picture

Status: Active » Needs review
FileSize
1.73 KB

straight forward

Status: Needs review » Needs work

The last submitted patch, test-access-1864218.1.patch, failed testing.

dawehner’s picture

Actually it should be

+++ b/core/modules/system/tests/modules/entity_test/lib/Drupal/entity_test/EntityTestAccessController.phpundefined
@@ -27,21 +27,21 @@ public function viewAccess(EntityInterface $entity, $langcode = LANGUAGE_DEFAULT
+   * Implements EntityAccessControllerInterface::createAccess().

Shouldn't it be even\Drupal\Core\Entity::EntityAccessControllerInterface()

larowlan’s picture

Status: Needs work » Needs review
FileSize
1.81 KB

sure!

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

This looks great!

fubhy’s picture

Status: Reviewed & tested by the community » Closed (works as designed)

Actually I am pretty sure that we agreed not to have full namespaces there. Which is why we removed them at some point in the other issue. @see http://drupal.org/coding-standards/docs#namespaces

larowlan’s picture

Status: Closed (works as designed) » Needs work

the method names are wrong eg they're ::viewAccess() not ::view()

larowlan’s picture

which is patch 1

larowlan’s picture

Status: Needs work » Needs review

#1: test-access-1864218.1.patch queued for re-testing.

larowlan’s picture

Please review patch 1

dawehner’s picture

I'm not sure as all new patches follow that standard at the moment (at least from my perspective).

Berdir’s picture

This is still being discussed back and forth in #1487760: [policy, no patch] Decide on documentation standards for namespaced items. Most patches that I've seen recently don't use the full name if it's in the same namespace. I currently don't really care about that standard, it has been changed a bunch times already and we have dozens of wrong classes that will need to be fixed anyway, doesn't matter what we decide to do now :) Using non-namespaces versions for things that are in the same namespace or use'd sounds fine to me.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

He, so all entity related patches work different to all CMI/VDC related patches :)

Let's RTBC patch #1

fubhy’s picture

Oh... Yeah +1 rtbc. I read the issue too quickly sorry. Patch #1 is okay!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

Status: Fixed » Closed (fixed)

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