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

Comments

berdir’s picture

berdir
Primary language German
Location Switzerland
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
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 commented
Status: Active » Needs review
StatusFileSize
new 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
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 🇪🇺🌍
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 commented
Status: Needs work » Needs review
StatusFileSize
new 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
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 🇪🇺🌍
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 commented
gsharm’s picture

gsharm commented
Assigned: Unassigned » gsharm
gsharm’s picture

gsharm commented
Assigned: gsharm » Unassigned
shamsher_alam’s picture

shamsher_alam commented
Status: Needs work » Needs review
StatusFileSize
new entity_render_cache_clear-2371397-15.patch0 bytes
shamsher_alam’s picture

shamsher_alam commented
StatusFileSize
new entity_render_cache_clear-2371397-15.patch557 bytes
mgifford’s picture

mgifford
he/him
Primary language English
commented
Status: Needs review » Needs work

Patch no longer applies.

wellme’s picture

wellme commented
StatusFileSize
new coding_style_fixes-2371397-18.patch3.47 KB
wellme’s picture

wellme commented
Status: Needs work » Needs review
timmillwood’s picture

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

alexpott
he/they
Primary language English
Location 🇪🇺🌍
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, stage 1 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
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.