This is not something that I have had the opportunity to test, but while looking through Drupal 8 Beta 2 code I noticed the following:

In \core\includes\entity.inc

function entity_render_cache_clear() {
  $entity_manager = Drupal::entityManager();  
  .....

Shouldn't it be "\Drupal::" with a back slash? Other functions in the entity include file reference Drupal container layer that way.

I'm sure that someone else will detect this soon enough. But since it is in an INCLUDE file, the developers/maintainers of the Entity system may not yet have seen it?

CommentFileSizeAuthor
#18 coding_style_fixes-2371397-18.patch3.47 KBwellme
#16 entity_render_cache_clear-2371397-15.patch557 bytesShamsher_Alam
#15 entity_render_cache_clear-2371397-15.patch0 bytesShamsher_Alam
#6 entity_render_cache_clear-2371397-6.patch2.31 KBadci_contributor
#3 entity_render_cache_clear-2371397-3.patch2.33 KBnikita.izotov
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Version: 8.0.0-beta2 » 8.0.x-dev
Priority: Normal » Minor

It could be, and there is a coding standard that says it should be there, but it is no functional bug.

Because the file itself is not in a namespace, so not explicitly specifying the namespace works fine.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Title: entity_render_cache_clear() function bug » entity_render_cache_clear() does not reference Drupal according to coding standards
nikita.izotov’s picture

nikita.izotov CreditAttribution: nikita.izotov commented
Status: Active » Needs review
FileSize
entity_render_cache_clear-2371397-3.patch2.33 KB

Fixed,
+ i've found some formating mistakes what've fixed also

FILE: /var/www/html/drupal/core/includes/entity.inc
--------------------------------------------------------------------------------
FOUND 6 ERROR(S) AND 1 WARNING(S) AFFECTING 7 LINE(S)
--------------------------------------------------------------------------------
  99 | ERROR   | Missing parameter type at position 2
 201 | ERROR   | Missing parameter type at position 1
 203 | ERROR   | Missing parameter type at position 2
 206 | ERROR   | Data type of return value is missing
 250 | WARNING | Line exceeds 80 characters; contains 83 characters
 255 | ERROR   | Missing parameter type at position 2
 260 | ERROR   | Data type of return value is missing
--------------------------------------------------------------------------------

Now sniffer says that "PHP_CodeSniffer did not find any problems."

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Title: entity_render_cache_clear() does not reference Drupal according to coding standards » Coding style fixes for includes/entity.inc
Status: Needs review » Reviewed & tested by the community

Changes look fine, but be careful with changing more than you have to in an issue. I think it's fine to expand the scope here, but you shouldn't do that if you are working on an actual change for example.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work 
  1. diff --git a/core/includes/entity.inc b/core/includes/entity.inc
old mode 100644
new mode 100755

    The file mode should not be changing 

  2. +++ b/core/includes/entity.inc
@@ -198,12 +198,12 @@ function entity_load_multiple_by_properties($entity_type, array $values) {
+ * @return array

    @return array|FALSE 

  3. +++ b/core/includes/entity.inc
@@ -247,17 +247,18 @@ function entity_create($entity_type, array $values = array()) {
+ * This is a wrapper for Drupal\Core\Entity\EntityInterface::label().
+ * This function should only be used as a callback, e.g. for menu title
+ * callbacks.

    There is no need to start each new sentence on a new line. 

  4. +++ b/core/includes/entity.inc
@@ -247,17 +247,18 @@ function entity_create($entity_type, array $values = array()) {
+ * @return string

    should be @return string|null

adci_contributor’s picture

adci_contributor CreditAttribution: adci_contributor commented
Status: Needs work » Needs review
FileSize
entity_render_cache_clear-2371397-6.patch2.31 KB

Fixed according to the recommendations from @alexpot

Status: Needs review » Needs work

The last submitted patch, 6: entity_render_cache_clear-2371397-6.patch, failed testing.

Status: Needs work » Needs review

the_contributor queued 6: entity_render_cache_clear-2371397-6.patch for re-testing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Status: Needs review » Reviewed & tested by the community

Patch in #6 has addressed #5.

jhedstrom queued 6: entity_render_cache_clear-2371397-6.patch for re-testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/includes/entity.inc
@@ -198,12 +198,12 @@ function entity_load_multiple_by_properties($entity_type, array $values) {
- * @return
+ * @return array|FALSE

Entities are not arrays.

gsharm’s picture

gsharm CreditAttribution: gsharm at Srijan | A Material+ Company commented
gsharm’s picture

gsharm CreditAttribution: gsharm at Srijan | A Material+ Company commented
Assigned: Unassigned » gsharm
gsharm’s picture

gsharm CreditAttribution: gsharm at Srijan | A Material+ Company commented
Assigned: gsharm » Unassigned
Shamsher_Alam’s picture

Shamsher_Alam CreditAttribution: Shamsher_Alam commented
Status: Needs work » Needs review
FileSize
entity_render_cache_clear-2371397-15.patch0 bytes
Shamsher_Alam’s picture

Shamsher_Alam CreditAttribution: Shamsher_Alam commented
FileSize
entity_render_cache_clear-2371397-15.patch557 bytes
mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented
Status: Needs review » Needs work

Patch no longer applies.

wellme’s picture

wellme CreditAttribution: wellme at Zyxware Technologies commented
FileSize
coding_style_fixes-2371397-18.patch3.47 KB
wellme’s picture

wellme CreditAttribution: wellme at Zyxware Technologies commented
Status: Needs work » Needs review
timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented
Version: 8.0.x-dev » 8.1.x-dev
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Needs review » Needs work

These kind of grabbag coding standards issues are discouraged because it is almost impossible to prevent regressions. If you interested in helping make core comply with coding standards please see #2571965: [meta] Fix PHP coding standards in core and the numerous sub issues there. Also it is worth reviewing https://www.drupal.org/core/scope for guidelines and examples for Drupal core issue scope.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs work » Closed (outdated)

entity.inc is now 100% deprecated, so I don't think we should spend time on fixing coding standard issues ther.