Postponed until 8.1.x and/or automated standards checking is on drupal.org.

Problem/Motivation

Some class files use the old standard of "Definition of Drupal..."

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task
Issue priority Minor because cosmetic.
Unfrozen changes Unfrozen (maybe) because it only changes documentation
Disruption NOT Disruptive for contributed and custom modules/themes because it will not require a BC break/deprecation/internal refactoring/widespread changes. But would disrupt patches in the core queue that would conflict with this.

Also, standards changes are to be postponed until 8.1.x and/or we have automatic testing for them. See #2350615-75: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?

Proposed resolution

Standard now is "Contains \Drupal..."

Do just field, in core/modules/field
Here is a grep that shows the filenames that need updated:

lib/Drupal/field/FieldException.php: * Definition of Drupal\field\FieldExeption.
lib/Drupal/field/FieldUpdateForbiddenException.php: * Definition of Drupal\field\FieldUpdateForbiddenException.
lib/Drupal/field/FieldValidationException.php: * Definition of Drupal\field\FieldValidationExeption.
lib/Drupal/field/Plugin/PluginSettingsBase.php: * Definition of Drupal\field\Plugin\PluginSettingsBase.
lib/Drupal/field/Plugin/PluginSettingsInterface.php: * Definition of Drupal\field\Plugin\PluginSettingsInterface.
lib/Drupal/field/Plugin/Type/Formatter/FormatterBase.php: * Definition of Drupal\field\Plugin\Type\Formatter\FormatterBase.
lib/Drupal/field/Plugin/Type/Formatter/FormatterFactory.php: * Definition of Drupal\field\Plugin\Type\Formatter\FormatterFactory.
lib/Drupal/field/Plugin/Type/Formatter/FormatterPluginManager.php: * Definition of Drupal\field\Plugin\Type\Formatter\FormatterPluginManager..
lib/Drupal/field/Plugin/Type/Widget/WidgetBase.php: * Definition of Drupal\field\Plugin\Type\Widget\WidgetBase.
lib/Drupal/field/Plugin/Type/Widget/WidgetBaseInterface.php: * Definition of Drupal\field\Plugin\Type\Widget\WidgetBaseInterface.
lib/Drupal/field/Plugin/Type/Widget/WidgetFactory.php: * Definition of Drupal\field\Plugin\WidgetFactory.
lib/Drupal/field/Plugin/Type/Widget/WidgetInterface.php: * Definition of Drupal\field\Plugin\Type\Widget\WidgetInterface.
lib/Drupal/field/Plugin/Type/Widget/WidgetPluginManager.php: * Definition of Drupal\field\Plugin\Type\Widget\WidgetPluginManager.
lib/Drupal/field/Plugin/views/argument/FieldList.php: * Definition of views_handler_argument_field_list.
lib/Drupal/field/Plugin/views/argument/ListString.php: * Definition of Drupal\field\Plugin\views\argument\ListString.
lib/Drupal/field/Plugin/views/field/Field.php: * Definition of Drupal\field\Plugin\views\field\Field.
lib/Drupal/field/Plugin/views/filter/FieldList.php: * Definition of Drupal\field\Plugin\views\filter\FieldList.
lib/Drupal/field/Plugin/views/relationship/EntityReverse.php: * Definition of Drupal\field\Plugin\views\relationship\EntityReverse.
lib/Drupal/field/Tests/ActiveTest.php: * Definition of Drupal\field\Tests\ActiveTest.
lib/Drupal/field/Tests/BulkDeleteTest.php: * Definition of Drupal\field\Tests\BulkDeleteTest.
lib/Drupal/field/Tests/CrudTest.php: * Definition of Drupal\field\Tests\CrudTest.
lib/Drupal/field/Tests/DisplayApiTest.php: * Definition of Drupal\field\Tests\DisplayApiTest.
lib/Drupal/field/Tests/FieldAccessTest.php: * Definition of Drupal\field\Tests\FieldAccessTest.
lib/Drupal/field/Tests/FieldAttachOtherTest.php: * Definition of Drupal\field\Tests\FieldAttachOtherTest.
lib/Drupal/field/Tests/FieldAttachStorageTest.php: * Definition of Drupal\field\Tests\FieldAttachStorageTest.
lib/Drupal/field/Tests/FieldInfoTest.php: * Definition of Drupal\field\Tests\FieldInfoTest.
lib/Drupal/field/Tests/FieldInstanceCrudTest.php: * Definition of Drupal\field\Tests\FieldInstanceCrudTest.
lib/Drupal/field/Tests/FieldTestBase.php: * Definition of Drupal\field\Tests\FieldTestBase.
lib/Drupal/field/Tests/FormTest.php: * Definition of Drupal\field\Tests\FormTest.
lib/Drupal/field/Tests/TranslationTest.php: * Definition of Drupal\field\Tests\TranslationTest.
lib/Drupal/field/Tests/TranslationWebTest.php: * Definition of Drupal\field\Tests\TranslationWebTest.
tests/modules/field_test/lib/Drupal/field_test/Plugin/field/widget/TestFieldWidget.php: * Definition of Drupal\field_test\Plugin\field\widget\TestFieldWidget.
tests/modules/field_test/lib/Drupal/field_test/Plugin/field/widget/TestFieldWidgetMultiple.php: * Definition of Drupal\field_test\Plugin\field\widget\TestFieldWidgetMultiple.
tests/modules/field_test/lib/Drupal/field_test/Plugin/field/widget/TestFieldWidgetNoDefault.php: * Definition of Drupal\field_test\Plugin\field\widget\TestFieldWidgetNoDefault.
tests/modules/field_test/lib/Drupal/field_test/TestEntityController.php: * Definition of Drupal\field_test\TestEntityController.
tests/modules/field_test/lib/Drupal/field_test/TestEntityFormController.php: * Definition of Drupal\field_test\TestEntityFormController.

Remaining tasks

Make the patch that updates just these files and just these lines to keep the patch reviewable.

User interface changes

None.

API changes

None.

Find the doc standards meta and link it here.
similar issue: #2002618: Definition of to Contains \Drupal in field_ui
#1310084: [meta] API documentation cleanup sprint

CommentFileSizeAuthor
#18 drupal-Definition_of_to_Contains_Drupal_in_field-2002624-18.patch19.13 KBmanu4543
#18 interdiff-14-18.patch2.32 KBmanu4543
#14 interdiff-12-14.txt3.03 KBshnark
#14 drupal-Definition_of_to_Contains_Drupal_in_field-2002624-14.patch20.74 KBshnark
#13 interdiff-3-12.txt1.83 KBshnark
#12 drupal-Definition_of_to_Contains_Drupal_in_field-2002624-12.patch17.71 KBshnark
#3 drupal-Definition_of_to_Contains_Drupal_in_field-2002624-2.patch18.83 KBatchijov

Comments

atchijov’s picture

atchijov commented

atchijov & johnbickar working on this.

atchijov’s picture

atchijov commented
Issue summary: View changes

Updated issue summary.

oenie’s picture

oenie commented

Try changing the 'assigned to' to one of your names.
That way people don't need to open the issue, because we can see it's assigned to someone !
Keep up the good work ! :)

atchijov’s picture

atchijov commented
Assigned: Unassigned » atchijov
Status: Active » Needs review
StatusFileSize
new drupal-Definition_of_to_Contains_Drupal_in_field-2002624-2.patch18.83 KB

Status: Needs review » Needs work
Issue tags: -Novice

The last submitted patch, drupal-Definition_of_to_Contains_Drupal_in_field-2002624-2.patch, failed testing.

oenie’s picture

oenie commented
Status: Needs work » Needs review

#3: drupal-Definition_of_to_Contains_Drupal_in_field-2002624-2.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, drupal-Definition_of_to_Contains_Drupal_in_field-2002624-2.patch, failed testing.

oenie’s picture

oenie commented
Status: Needs work » Needs review
Issue tags: +Novice

#3: drupal-Definition_of_to_Contains_Drupal_in_field-2002624-2.patch queued for re-testing.

atchijov’s picture

atchijov commented

@oenie I see that patch is marked as "PASSED" but at the same time I see number of comments about path "failed testing" / "queued for re-testing". Am I correct that these failures were due to problems with testing system and that the patch IS ok?

zakiya’s picture

zakiya commented
Assigned: atchijov » Unassigned
Status: Needs review » Reviewed & tested by the community

Patch modifies documentation as stated, applies and has been tested. Earlier fails were probably due to issues with the test-bot.

yesct’s picture

yesct commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetFactory.phpundefined
@@ -15,7 +15,7 @@
   /**
-   * Overrides Drupal\Component\Plugin\Factory\DefaultFactory::createInstance().
+   * Overrides \Drupal\Component\Plugin\Factory\DefaultFactory::createInstance().
    */
   public function createInstance($plugin_id, array $configuration) {

issue is just about changing Contains lines.

Overrides is using {@inheritdoc} now.
Let's skip this change please, and do that in another issue. See #1906474: [policy adopted] Stop documenting methods with "Overrides …"

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetInterface.phpundefined
@@ -14,9 +14,9 @@
- * to override. See Drupal\field\Plugin\Type\Widget\WidgetBaseInterface for base
+ * to override. See \Drupal\field\Plugin\Type\Widget\WidgetBaseInterface for base
  * wrapping methods that should most likely be inherited directly from
- * Drupal\field\Plugin\Type\Widget\WidgetBase..
+ * \Drupal\field\Plugin\Type\Widget\WidgetBase..

Yes, name spaces in comments should be fulling qualified with a leading slash. But, that's not what this issue is about.

Also, it seems like a few files have been missed. To check, do another grep.

shnark’s picture

shnark commented
Assigned: Unassigned » shnark

I'm going to start working on this.

shnark’s picture

shnark commented
StatusFileSize
new drupal-Definition_of_to_Contains_Drupal_in_field-2002624-12.patch17.71 KB

I took out the changes that were not part of the issue.
in core/modules/field i did:

$ grep -R "Definition of \Drupal" *

and it came up with some results.

lib/Drupal/field/FieldException.php: * Definition of Drupal\field\FieldExeption.
lib/Drupal/field/FieldUpdateForbiddenException.php: * Definition of Drupal\field\FieldUpdateForbiddenException.
lib/Drupal/field/FieldValidationException.php: * Definition of Drupal\field\FieldValidationExeption.
lib/Drupal/field/Plugin/PluginSettingsBase.php: * Definition of Drupal\field\Plugin\PluginSettingsBase.
lib/Drupal/field/Plugin/PluginSettingsInterface.php: * Definition of Drupal\field\Plugin\PluginSettingsInterface.
tests/modules/field_test/lib/Drupal/field_test/TestEntityFormController.php: * Definition of Drupal\field_test\TestEntityFormController.

and

$ grep -R "Definition of Drupal" *

lib/Drupal/field/FieldException.php: * Definition of Drupal\field\FieldExeption.
lib/Drupal/field/FieldUpdateForbiddenException.php: * Definition of Drupal\field\FieldUpdateForbiddenException.
lib/Drupal/field/FieldValidationException.php: * Definition of Drupal\field\FieldValidationExeption.
lib/Drupal/field/Plugin/PluginSettingsBase.php: * Definition of Drupal\field\Plugin\PluginSettingsBase.
lib/Drupal/field/Plugin/PluginSettingsInterface.php: * Definition of Drupal\field\Plugin\PluginSettingsInterface.
tests/modules/field_test/lib/Drupal/field_test/TestEntityFormController.php: * Definition of Drupal\field_test\TestEntityFormController.

so I'm going to change those to Contains \Drupal.

shnark’s picture

shnark commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-3-12.txt1.83 KB

Sorry, I hit save accidentally.
Here is the interdiff.
Next I'm going to change the things I found with grep.

shnark’s picture

shnark commented
Assigned: shnark » Unassigned
StatusFileSize
new drupal-Definition_of_to_Contains_Drupal_in_field-2002624-14.patch20.74 KB
new interdiff-12-14.txt3.03 KB

I changed the things that showed up when I did:
$ grep -R "Definition of Drupal" *
by doing this:
find . -name "*.php" -exec sed -i "" 's/Definition of Drupal/Contains \\Drupal/g' '{}' \;

Since I'm on a mac, I had to put "" after the -i.

I had 2 groups of files in comment #12 but they are actually the same,
since I did:
grep -R "Definition of \Drupal" *
If I wanted to find the ones with "Definition of \Drupal" I would have needed to \\.
I did grep -R "Definition of \\Drupal" * there weren't any files.
it might be useful for anybody reviewing this to use git diff --color-words

yesct’s picture

yesct commented 
+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetBaseInterface.phpundefined
@@ -27,7 +27,7 @@
-   * @param Drupal\Core\Entity\EntityInterface $entity
+   * @param \Drupal\Core\Entity\EntityInterface $entity

@@ -50,7 +50,7 @@ public function form(EntityInterface $entity, $langcode, array $items, array &$f
-   * @param Drupal\Core\Entity\EntityInterface $entity
+   * @param \Drupal\Core\Entity\EntityInterface $entity

@@ -68,7 +68,7 @@ public function extractFormValues(EntityInterface $entity, $langcode, array &$it
-   * @param Drupal\Core\Entity\EntityInterface $entity
+   * @param \Drupal\Core\Entity\EntityInterface $entity

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetFactory.phpundefined
@@ -15,7 +15,7 @@
-   * Overrides Drupal\Component\Plugin\Factory\DefaultFactory::createInstance().
+   * Overrides \Drupal\Component\Plugin\Factory\DefaultFactory::createInstance().

These should be taken out also.

yesct’s picture

yesct commented
Status: Needs review » Needs work
manu4543’s picture

manu4543 commented
Assigned: Unassigned » manu4543
manu4543’s picture

manu4543 commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-14-18.patch2.32 KB
new drupal-Definition_of_to_Contains_Drupal_in_field-2002624-18.patch19.13 KB

Removed out of scope changes.

Status: Needs review » Needs work
Issue tags: -Novice

The last submitted patch, interdiff-14-18.patch, failed testing.

manu4543’s picture

manu4543 commented
Status: Needs work » Needs review
Issue tags: +Novice

#18: drupal-Definition_of_to_Contains_Drupal_in_field-2002624-18.patch queued for re-testing.

yesct’s picture

yesct commented

@manu4543 I looked at the interdiff and that look great.
The patch also looks good.

I tried to apply it locally and got some hunk errors, but when I looked manually at what it was trying to do, the files looked like they should have been found, so lets see what that bot says, see if it looks like it's able to apply there.

When making interdiffs, use the .txt extension instead of .patch, the testbot will ignore all .txt files, and we dont want it testing interdiffs. :)

If it comes back green I would (or anyone) like to apply it locally and do a grep for Definition of to make sure we got them all and no new ones have snuck in.

yesct’s picture

yesct commented
Status: Needs review » Reviewed & tested by the community

I did a sudo rm -r on my drupal git webroot
and a reclone,
then downloaded the patch and applied manually (instead of drush am)
and it applies clean.

then:
$ grep -R "Definition of " core/modules/field/*
[~/foo/d8-patch]
08:18 AM [YesCT] (8.x)
547 $

So looks great! RTBC

alexpott’s picture

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

the field module is changing a lot at the moment I would like to postpone this clean up till after the fieldapi and entityng are tied together... #1949932: [META] Unify entity fields and field API

Have a look at the conflict...

curl https://drupal.org/files/drupal-Definition_of_to_Contains_Drupal_in_field-2002624-18.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 19584  100 19584    0     0  12599      0  0:00:01  0:00:01 --:--:-- 14474
error: patch failed: core/modules/field/lib/Drupal/field/FieldUpdateForbiddenException.php:2
error: core/modules/field/lib/Drupal/field/FieldUpdateForbiddenException.php: patch does not apply
error: patch failed: core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterBase.php:2
error: core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterBase.php: patch does not apply
error: core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterFactory.php: does not exist in index
error: patch failed: core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetBase.php:2
error: core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetBase.php: patch does not apply
error: patch failed: core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetFactory.php:2
error: core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetFactory.php: patch does not apply
error: core/modules/field/tests/modules/field_test/lib/Drupal/field_test/TestEntityController.php: does not exist in index
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes

Updated issue summary. Added quotation marks for clarity.

Status: Postponed » Needs review

mgifford queued 18: drupal-Definition_of_to_Contains_Drupal_in_field-2002624-18.patch for re-testing.

mgifford’s picture

mgifford
he/him
Primary language English
commented
Related issues: +#1949932: [META] Unify entity fields and field API

It was RTBC, but postponed on an issue which is now in Core.

Status: Needs review » Needs work

The last submitted patch, 18: drupal-Definition_of_to_Contains_Drupal_in_field-2002624-18.patch, failed testing.

yesct’s picture

yesct commented
Version: 8.0.x-dev » 8.1.x-dev
Assigned: manu4543 » Unassigned
Issue summary: View changes
Status: Needs work » Postponed
Issue tags: -Novice +Coding standards

removing novice tag per https://www.drupal.org/core-mentoring/novice-tasks

a bug with a novice task will be much more likely to get reviewed at this point. try one of:
https://www.drupal.org/project/issues/search/drupal?project_issue_follow...

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

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.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.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now 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.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Postponed » Closed (won't fix)

We no longer use @file with 'Contains..' language.

https://www.drupal.org/docs/develop/coding-standards/api-documentation-a...