Minestatus

There's a lot of cruft left over from the Wordpress version in this...

- Both the docblocks at the top of the module file are nonstandard.

> return array('view server credentials');

That's not a very clear permission name. Which server?

- hook_block keeps switching on $delta when there is only one!

/**
 * Get the Minecraft server host
 */
function _minestatus_get_host() {
  return variable_get('minestatus_host', '');
}

You have a lot of functions like this which just wrap variable_get or variable_set. I don't see the point of these at all.

    if (_minestatus_get_port_visibility() && user_access('view server credentials')) {
      array_unshift($stats_list, t('Server port:') . ' ' . $stats['server_port']);
    }

Why unshift? Why not put that in the array earlier on, since you're the one creating it?

description = "This plugin shows several server parameters using the Minequery plugin (http://minestatus.net/minequery). Returned parameters are: load time, server port, maximum players and player-list."

Plugin, eh?

version     = "6.x-1.2"
package     = "Minestatus"

Version is added for you automatically. Package should only be set when you're forming part of an established group, so should not be there.

<div id="minestatus">
  <?php print $minestatus_list; ?>
  <small><?print t('Last update @updated', array('@updated' => $last_update)); ?></small>
</div>

If that's all the theming you have to do, then you really don't need a TPL file. You should be using a theme function instead, which should also have the stuff from minestatus_preprocess_minestatus_list() in it.

Finally, run this through Coder. I don't see many formatting errors, but I've spotted at least one :)

> I did got a warning of a missing $Id$ comment in Coder on the .install file, is this still necessary in Git?

That's a bug in Coder -- the $Id$ strings should definitely not be there for Git :)

Marking as critical as per http://drupal.org/node/894256
Sorry for the long wait.

Some comments inline:

  $blocks = array(); // you don't need to set this outside the 'list' op
  if ('list' == $op) {
      $blocks[0] = array( // indentation wrong here.
        'info' => t('Minestatus'),
        'cache' => BLOCK_NO_CACHE,
      );
      return $blocks;
  }

    $start = microtime(true);

Capitalise constants: TRUE.

> fwrite($obj_sock, 'QUERY_JSON'."\n");

Space either side of . operator.

Missing newline at end of module file.

    'minestatus_list' => array(
      'arguments' => NULL,
    ),

'arguments' should probably be an empty array rather than NULL? Not sure though.

> '#title' => t('Cache the results for a period in seconds'),

This should describe the field rather than the action, so 'Duration for which the results are cached, in seconds.' would be better.

> if ('list' == $op) {

I find that way round of writing an equality test a bit odd to read, but that's just personal preference; AFAIK there's no standards on it.

Just minor niggles overall, so I'm happy to mark this RTBC as I'm confident these will be addressed :)

Here's a final coding standards patch for you, but I've also given you full project creation rights. Thank you for your patience!