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.

CommentFileSizeAuthor
fix_newlines_file_inc.patch7.81 KBaspilicious

Comments

quicksketch’s picture

quicksketch
he/him
commented

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.

aspilicious’s picture

aspilicious commented

Well maybe I can make this the fix newline issue. After all the critical patches are in...

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

I don't see any problems with the above patch.

lotyrin’s picture

lotyrin commented

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?

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Alrighty, committed to CVS HEAD.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.