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
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.
Related Issues
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
Comment | File | Size | Author |
---|---|---|---|
#18 | drupal-Definition_of_to_Contains_Drupal_in_field-2002624-18.patch | 19.13 KB | manu4543 |
#18 | interdiff-14-18.patch | 2.32 KB | manu4543 |
#14 | interdiff-12-14.txt | 3.03 KB | shnark |
#14 | drupal-Definition_of_to_Contains_Drupal_in_field-2002624-14.patch | 20.74 KB | shnark |
#13 | interdiff-3-12.txt | 1.83 KB | shnark |
Comments
Comment #1
atchijov CreditAttribution: atchijov commentedatchijov & johnbickar working on this.
Comment #1.0
atchijov CreditAttribution: atchijov commentedUpdated issue summary.
Comment #2
oenie CreditAttribution: oenie commentedTry 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 ! :)
Comment #3
atchijov CreditAttribution: atchijov commentedComment #5
oenie CreditAttribution: oenie commented#3: drupal-Definition_of_to_Contains_Drupal_in_field-2002624-2.patch queued for re-testing.
Comment #7
oenie CreditAttribution: oenie commented#3: drupal-Definition_of_to_Contains_Drupal_in_field-2002624-2.patch queued for re-testing.
Comment #8
atchijov CreditAttribution: 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?
Comment #9
zakiya CreditAttribution: zakiya commentedPatch modifies documentation as stated, applies and has been tested. Earlier fails were probably due to issues with the test-bot.
Comment #10
YesCT CreditAttribution: YesCT commentedissue 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 …"
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.
Comment #11
shnark CreditAttribution: shnark commentedI'm going to start working on this.
Comment #12
shnark CreditAttribution: shnark commentedI 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.
Comment #13
shnark CreditAttribution: shnark commentedSorry, I hit save accidentally.
Here is the interdiff.
Next I'm going to change the things I found with grep.
Comment #14
shnark CreditAttribution: shnark commentedI 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
Comment #15
YesCT CreditAttribution: YesCT commentedThese should be taken out also.
Comment #16
YesCT CreditAttribution: YesCT commentedComment #17
manu4543 CreditAttribution: manu4543 commentedComment #18
manu4543 CreditAttribution: manu4543 commentedRemoved out of scope changes.
Comment #20
manu4543 CreditAttribution: manu4543 commented#18: drupal-Definition_of_to_Contains_Drupal_in_field-2002624-18.patch queued for re-testing.
Comment #21
YesCT CreditAttribution: 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.
Comment #22
YesCT CreditAttribution: YesCT commentedI 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
Comment #23
alexpottthe 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...
Comment #23.0
alexpottUpdated issue summary. Added quotation marks for clarity.
Comment #25
mgiffordIt was RTBC, but postponed on an issue which is now in Core.
Comment #27
YesCT CreditAttribution: YesCT commentedremoving 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...
Comment #30
Mile23We no longer use @file with 'Contains..' language.
https://www.drupal.org/docs/develop/coding-standards/api-documentation-a...