Posted by aspilicious on July 3, 2010 at 6:31pm
6 followers
Jump to:
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | documentation |
| Category: | bug report |
| Priority: | normal |
| Assigned: | aspilicious |
| Status: | closed (fixed) |
Issue Summary
This issue is *only* for fixing the newlines.
Don't commit this one before #353458: hook_file_references() was not designed for a highly flexible field storage is in, that one is critical and this one will need a reroll afterwards.
If this issue is fixed I'll open one for fixing the style issues that are left.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| fix_newlines_file_inc.patch | 7.81 KB | Idle | PASSED: [[SimpleTest]]: [MySQL] 22,018 pass(es). | View details |
Comments
#1
Do you have any inclination to fix this problem for other files too? The following are mixed or all one way or the other:
Mixed:
form.inc
actions.inc
bootstrap.inc
cache.inc
common.inc
errors.inc
form.inc
locale.inc
mail.inc
menu.inc
module.inc
path.inc
theme.inc
unicode.inc
New lines:
ajax.inc
entity.inc
install.core.inc
language.inc
password.inc
stream_wrappers.inc
update.inc
updater.inc (this one also has all kinds of extra data on return and param properties: bool, array, etc.)
No new lines:
archiver.inc
authorize.inc
batch.inc
image.inc
install.inc
lock.inc
session.inc
tablesort.inc
token.inc
xmlrpc.inc
xmlrpcs.inc
If I were to try and discern a pattern here, it seems that most of our older code does not include a newline before @return, while some of our newer code does include newlines. It may just be a matter of who wrote the patch and what their preference was.
Ah, I just found the issue. Our "standard" was changed not to long ago: See #550942: Blank line between @param and @return. Unfortunately after the change, none of the code was actually updated. Funny the ways we find to fill up our development time.
#2
Well maybe I can make this the fix newline issue. After all the critical patches are in...
#3
I don't see any problems with the above patch.
#4
I don't think this issue should fix all of the documentation in core. That would quickly get out of hand, having to re-roll, re-test, and re-review a very large (by my estimates > 1000 line) patch.
Perhaps we should create a meta-issue?
#5
Alrighty, committed to CVS HEAD.
#6
Automatically closed -- issue fixed for 2 weeks with no activity.