Compare D7 hook_node_load with D6 hook_nodeapi(op=load)

Just noticed another improvement:

• 4. instead of doing the foreach ($nodes as $node) loop, we can just use array_keys($nodes) instead of $nids in the SQL.

Yes I agree with leaving 1 and 2 for later.
In your patch, I would do it in one line, ie change

  $nids = array_keys($nodes);
  $result = db_query('SELECT * FROM {scheduler} WHERE nid IN (:nids)', array(':nids' => $nids));

into

  $result = db_query('SELECT * FROM {scheduler} WHERE nid IN (:nids)', array(':nids' => array_keys($nodes)));

to save the overhead of creating the array $nids which is not used anywhere else. This is how it is done in the API docs example

Yes, that is very good reasoning. In addition, having it as two lines means you can insert a debug line between the two if necessary. I'll test the patch and report back.

Your patch is fine, but I think there was another error in the original translation to D7. When $row was created in D6 it was being saved to $node->scheduler which is correct because the D6 version can only load one node at a time. But in D7, multiple nodes can be loaded, and they are stored in $nodes keyed by the node id. So the line

$node->scheduler = $row;

should become

$nodes[$nid]->scheduler = $row;

Otherwise the $row never gets saved and stored, which is half of the point of this function.

Also you could change the function header text to read 'Implements hook_node_load().' as this is the new standard. I know this is quite minor, but this header block is bound to be copied to the other new hook_node_{op} functions so we might as well save ourselves effort later on.

Forgot to set status back to 'needs work'

Yes the actual values get stored in $nodes[$nid]->publish_on, that is fine. What I'm saying is that the formatted text (which used to be stored in $node->scheduler via $row in D6) is now not saved at all. To match D6 we need the line change I suggested in #7 above.

I've just looked back, and the array $node->scheduler actually has all four scheduler values in the D5 and D6 versions. See screen grab from D6. The ->scheduler array was introduced somewhere between 5.x-1.3 (March 2007) and 5.x-1.10 (Feb 2008). I don't know the background why publish_on and unpublish_on are duplicated directly in $node and in the $node->scheduler array but I don't think we should remove either of them in the D7 conversion (atleast not for 7.x-1.0). They are not accessed in any scheduler module code, just provided for others to use, I presume.

So in addition to the line change in #7 above, we also need

    $row['publish_on'] = $record->publish_on;
    $row['unpublish_on'] = $record->unpublish_on;

so that the stored array has all four values just like D6.