Blank line between @param and @return
catch - August 17, 2009 - 00:37
| Project: | Coder |
| Version: | 7.x-1.x-dev |
| Component: | Review/Rules |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
| Issue tags: | coding standards, DX (Developer Experience), Novice |
Jump to:
Description
<?php
/**
* Load node objects from the database.
*
* This function should be used whenever you need to load more than one node
* from the database. Nodes are loaded into memory and will not require
* database access if loaded again during the same page request.
*
* @param $nids
* An array of node IDs.
* @param $conditions
* An array of conditions on the {node} table in the form 'field' => $value.
* @param $reset
* Whether to reset the internal node_load cache.
*
* @return
* An array of node objects indexed by nid.
*/
?><?php
/**
* Generate an array which displays a node detail page.
*
* @param $node
* A node object.
* @param $message
* A flag which sets a page title relevant to the revision being viewed.
* @return
* A $page element suitable for use by drupal_page_render().
*/
?>We have both in core, someone told me to remove the preceding line break in one of my patches, and I assumed it was standard in core so have been passing that around, but clearly we're inconsistent. Would be good to settle one way or the other.
#1
#2
An additional blank line between all @param and @return directives helps in readability, especially when quickly scanning code or a patch.
I don't know who advocated the other way, but it doesn't make sense to me.
#3
I prefer the blank line, and I actually thought that was the standard and was surprised when I was correct.
#4
I think we have an agreement.
#5
Looks good to me. Uh. I think this is appropriate?
#6
Ummm.
I think not having a separate blank line has been the standard for a long time. See:
http://drupal.org/node/1354
That is where it should be updated, if everyone has agreed it should be changing. Has "everyone" actually agreed?
#7
"Documentation" project?! No wonder I didn't find this issue anymore. Searched countless of times for it. :-/
For now, let's put it back to Drupal. When there is an agreement (and ideally, core has been patched accordingly), it should move to Coder.
That said, I think we have an agreement already.
#8
#9
FYI: I updated http://drupal.org/node/1354 (diff)
#10
So the standard is updated. Should we mark this issue fixed, or do you want the entire core code base to be patched before it is marked fixed?
#11
Yeah, we should definitely fix core. A nice little task. However, we should wait until after API freeze, since a patch like that would break every other in the queue ;)
#12
When is API freeze? I thought the code freeze as of Sept 1 was the API freeze, aside from exceptions blessed by webdries?
#13
October 15: http://drupal.org/node/578446. Between Sept freeze and the Oct freeze, new features need to be blessed by webdries, but API changes to implement existing features cleaner is still open.
#14
There are more standards than this one that are violated by various functions in Drupal 7. A few months back, I tried to get some patches in to fix things, file by file, but someone insulted me as being too picky (which I didn't appreciate at ALL), and I gave up trying to do wholesale patches that brought large sections of doc into compliance with the standards we've agreed upon.
So I think that we should just go ahead and mark this issue (of figuring out what the standards should be and documenting them, which has been done) as Fixed, and then the doc can slowly brought into compliance with the standards as time goes on.
#15
Next home for this issue is Coder module to ensure that this rule is adhered to.