Field: Trivial cleanup

Let's do this the easy way. A CVS patch can't handle file adds or removes. So, Dries or webchick, please:

cd modules/field
mv api.field.php field.api.php
cvs remove api.field.php
cvs add field.api.php

rm sample_code.php
cvs remove sample_code.php
cvs commit

Then set this back to CNW and we'll fix up the missing CVS Id tags.

PIFR parses the patches for the list of files to lint. I suspect it can't handle a file being removed by a patch and that no longer exists, similar issue #366952: "Invalid PHP Syntax" when testing binary files that don't exist

#9 has been taken care of. Marking to CNW per Barry.

This patch adds $Id$ tags to all files in module/field that are missing them. I am foolishly marking my own comment-only patch RTBC. :-)

Note that a previous patch eliminated all double spacing between code comments in core in favor of single spacing, and added a standard for single spacing between comments to the coding standards page.

However, no reason to let that stop this -- I'll roll a patch to eliminate all the double spaces that fields-in-core patches have reintroduced in another issue.

Well. If we're going to do a trivial clean-up, let's do it right.

1. field.info.inc also needs a @file block.
2. The space needs to be removed between the field_default_validate() PHPDoc and its function declaration, or it will not show up in api.drupal.org. And why are we removing comments there?
3. field.crud.inc also needs a @file block.

Angie, I'm getting overwhelmed with multiple patches to the same files in module/field. Could you make another interim commit of the $Id$-only patch in #12, and then we'll continue the other "trivial fixes" in this issue with separate patches in this same issue?

In that case, re-rolled with TODOs in place, and committed this to HEAD.

Setting back to 'active' though, since from a quick glance-through, we are not even remotely close to done with cleanup in this series of files.

I scanned through the comment changes and saw no obvious problems. Also, none of the tests broke. :-)

Ok, cool. Committed this one, and marking back to active for a few remaining items I found as I was picking through. Since everything other than the last item can be done by someone who is not bjaspan/yched/etc. and would be great to get some helping hands in here by people OTHER than them so that they can focus on more critical stuff. I've thus tagged this issue as "novice" to hopefully attract someone to work on these few remaining minor items.

+  // hook_field_validate. NOTE: This tests the FAPI-connected
+  // behavior of hook_field_validate. As discussed at

Let's change this NOTE to a TODO, and put it in its own line so that we can find it later.

 // $Id: field.install,v 1.1 2009/02/03 17:30:11 dries Exp $

Missing @file. Though on quick glance, it looks like every other install file is also missing @file, so you can skip this. Tagged #320011: Add doxygen @file for all .install as "novice" to hopefully get some eyes on it.

 // TOTO D7 : consistency - some of these functions process individual fields

TODO. :)

+ * See field_attach_load() for details and arguments. TODO:
  * Currently, this hook only accepts a single object a time.

TODO needs to be moved to a separate line. Otherwise IDEs see "TODO: [blank]" as the item, rather than "TODO: Currently, this hook..."

Just about every function in field.api.php (and probably elsewhere) has an initial PHPDoc line longer than 80 characters. These should be split into a single-line description and then secondary description of unlimited length. For example:

 /**
  * Define custom delete behavior for this module's field types. This
  * hook is invoked just before the data is deleted from field storage.
  *
  * @param $obj_type

Should become:

 /**
  * Define custom delete behavior for this module's field types.
  *
  * This hook is invoked just before the data is deleted from field storage.
  *
  * @param $obj_type

Now, something that we need "Fields in Core Sprint Attendees" knowledge on is that some of your TODOs have D7 after them and some of them don't. Is this because they were done by different people, or do all of the TODOs without D7 after them indicate "sometime well in the future"?

Field API started life as content.module as associated files. Those files already had TODOs in them. We sort of followed the practice of writing "TODO D7" for new TODOs added during the sprint. I am not sure if we have stuck with that convention. Also, probably a lot of the TODOs from CCK are no longer relevant, or should really be fixed now instead of being left as TODOs forever. So, I don't think we need to make a point of tracking down the differences.

Ok, thanks! Then sometime before we close this ticket "for good," it would be wonderful to have a fields in core sprint attendee do a "TODO" pass and just make sure the list matches our actual remaining todos. Everything else can be done by our newer contributors. :)

Addressed webchick's issues in #23 (except the doxygen .install @file issue that's offloaded to #320011, and the question about TODOs with and without "D7"...)

Yay! Thanks a lot. :) Committed to HEAD.

I think that's cleaned up enough to call this issue fixed for now. ;)