Posted by pcambra on January 23, 2013 at 10:26pm
7 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | documentation |
| Category: | task |
| Priority: | normal |
| Assigned: | rteijeiro |
| Status: | postponed |
| Issue tags: | Novice |
Issue Summary
As for this documentation http://drupal.org/node/1354#param-return-data-type and #1892472: Document hook_block_access we should:
- Replace all references to "boolean" and "Boolean" by "bool" in "@var", "@param" and "@return" directives.
- Replace all references to "Array" by "array" in "@var", "@param" and "@return" directives.
- Replace all references to "integer" and "Integer" by "int" in "@var", "@param" and "@return" directives.
- Replace leading slash before these three directives for strings like '@param Drupal\' and '@param Symphony'.
- A global clean up of 'Contains ' replacing 'Definition of ' in the core class files.
Comments
#1
Probably a good Novice project.
#2
pcambra I am working on it ;)
#3
#4
Also replaced "@var boolean" declarations.
Attached complete patch. Hope it's right.
#5
exclude 3rd party scripts ?
+++ b/core/vendor/doctrine/common/lib/Doctrine/Common/EventManager.phpundefined
--- a/core/vendor/doctrine/common/lib/Doctrine/Common/Lexer.php
+++ b/core/vendor/doctrine/common/lib/Doctrine/Common/Lexer.phpundefined
+++ b/core/vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/Request.phpundefined
--- a/core/vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/Resources/stubs/SessionHandlerInterface.php
+++ b/core/vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/Resources/stubs/SessionHandlerInterface.phpundefined
#6
Despite not living in core/vendor ArchiveTar is also an external file and should be excluded as well.
#7
I understood that these type of patches that touch many files were not going to be committed until after the new feature deadline in February. Good to see this one being developed in anticipation.
It looks like you missed the '@var Boolean' in ..\Core\Database\Transaction.php. I did not see any others in a quick scan.b
As a follow up to this issue, other "basic" house-cleaning documentation issues like this one include:
- 'array' instead of 'Array' in @return, @param and @var directives,
- 'int' instead of 'integer' in @return, @param and @var directives,
- leading slash before these three directives for strings like '@param Drupal\' and '@param Symphony', and
- a global clean up of 'Contains ' replacing 'Definition of ' in the core class files.
#8
Updating issue title
#9
Let's leave this issue just for "boolean", and leave the issue title as it was. Separate issues can be filed for the other issues if necessary.
And yes, as mentioned above (sorry, forgot to think of this): Definitely don't touch any files with /vendor/ in their path, or ArchiveTar, which should be in /vendor/ but isn't (don't even ask).
#10
A somewhat related issue is the use of the lower case term 'boolean' in the @param/return/var directive descriptions. According to #1431632-10: Clean up API docs for translation module, @xjm and @jhodgdon have previously stated that 'the word "Boolean" should always be capitalized (because it's derived from the name Boole).'
I am not sure whether this 'boolean' capitalization issue also should be dealt with in this issue or a follow-up issue.
#11
Let's keep this issue targeted.
And you're probably right that this issue should be postponed until after feature freeze, when we'll enter the "cleanup" phase.
#12
Related issue: #1860690: Correct Boolean use in docblocks
#13
Hey what's up with all the changes I have done?
So if you want I could keep them in a branch and merge them later or maybe I could do something else.
#14
If you want to make a patch that omits the vendor files and ArchiveTar, we can revive it or reroll it when it's time to un-postpone this issue in a few weeks. Thanks! And sorry about forgetting we aren't supposed to do massive cleanups before then...