Anybody using this module successfully?

Comments

danielb’s picture

It's just that I've had no feedback about the project so I'm not sure what my next move should be

Have you installed Beautifier on your site? Why? Did it do what you thought it would? Will you continue to use this module? Do you plan to customise your own presets? Have you encountered any problems?

danielb’s picture

oh come on, at least 7 people installed this module since I posted this....

danielb’s picture

Status: Active » Closed (fixed)

okidoki

dman’s picture

Status: Closed (fixed) » Needs work

I've not installed it yet, but am just looking at the online demo - thanks heaps for making THAT available!

First impressions:
It converted

  $command = drush_get_command();
  $settings = sitesynch_get_site_settings();
  $element = array_shift($command['arguments']);
  $backupfilepath = array_shift($command['arguments']);

into

  $command        = drush_get_command();
  $settings       = sitesynch_get_site_settings();
  $element        = array_shift($command['arguments']);
  $backupfilepath = array_shift($command['arguments']);

... which made me celebrate!

... but then it converted

      $settings['DB_DRIVER']   = $creds['driver'];
      $settings['DB_HOST']     = $creds['host'];
      $settings['DB_USERNAME'] = $creds['user'];

down to :

      $settings['DB_DRIVER'] = $creds['driver'];
      $settings['DB_HOST'] = $creds['host'];
      $settings['DB_USERNAME'] = $creds['user'];

... which made me sad again.

I'm not keen on indenting the closing ) in arrays and things.
Mine:

  $excludes = array(
    '.DS_Store',
    '.svn',
  );

becomes:

  $excludes = array(
    '.DS_Store',
    '.svn',
    );

... but I know that's maybe a debatable preference.
I like it because it REALLY helps visual nesting matches.

  if (!empty($enabled_paths)) {
    foreach ($enabled_paths as $path) {
      sitesynch_exec_rsync(
        $settings,
        $peer_settings,
      );
    }
  }

Check the closing braces, vs the unbalanced 'beautified':

  if (!empty($enabled_paths)) {
    foreach ($enabled_paths as $path) {
      sitesynch_exec_rsync(
        $settings,
        $peer_settings,
        );
    }
  }

:(

...
OK, something went wrong here:
Input:

/*
 * Sample REQUIRED settings when talking to a peer. Add these to settings.php
 */
$all_settings[$SERVERNAME] = array(
  'LABEL'             => "Mother Dev",  # Human label for this instance
  'PEER'              => "actualpixel",      # Use the `hostname` the other server identifies itself as
  'SSH_USERNAME'      => "dan",
);

'Beautified':

/*
 * Sample REQUIRED settings when talking to a peer. Add these to settings.php
 */

$all_settings[$SERVERNAME] = array(
  # Human label for this instance
  # Use the `hostname` the other server identifies itself as
  
  'SSH_USERNAME' => "dan",
  );

!! That actually destroyed some code, as well as added some extra lines.

I know that using # for comments may not be supported, but I use # for scaffolding and debug code, and // for persistent real comments. # is a legal comment.

... anyway, this was just a once-over review on one file based on the published demo. I'd LIKE it if this could be used to fix up all those whitespace complaints that coder.module has (boring!) but I'm not sure I can trust it just yet...

Feedback you wanted?

NancyDru’s picture

I'm checking it out now. Perhaps you should run it on itself - lots of messages from Coder.

Please change the "package" to "Development" so it goes in the same group as Devel.

NancyDru’s picture

Before I could see anything, it went away. Where did the results go?

danielb’s picture

Thanks guys! I will run it on itself when it is trustworthy enough I guess. Thanks for the samples - I can use those to debug what the algorithm thought it was doing there.

What went away?

There are some usability issues with customising your own format too - you have to enter a name, hit save, and then choose your new preset from the list and hit 'use selected preset' before you can actually modify it properly.... should make that more intuitive.

Also a lot of the subfunctions don't have a description and so the function name is shown in the interface.

NancyDru’s picture

Whatever messages if tried to throw up on the screen disappeared faster than I could see them.

Also, please change the package to "Development" so it goes in the same group as Coder.

danielb’s picture

This module is not meant to be in a package with coder, please see http://drupal.org/node/536640

Whatever messages if tried to throw up on the screen disappeared faster than I could see them.

I am not aware of such functionality? It is possible you were looking at output from a different module being tested on the same site at the time. :/

NancyDru’s picture

As a guideline, four or more modules that depend on each other (or all on a single module) make a good candidate for a package. Fewer probably do not.

If used, the package string is used to group modules together on the module administration display; the string should therefore be the heading you would like your modules to appear under...

from http://drupal.org/node/231036

I would prefer to see this moved under the other heading rather than having an entirely new one (for only two modules). It has nothing to do, really, with being in a package with Coder, any more than with Devel or Helpers, which also use that package name. The modules admin page is slow enough without an extra package group.

danielb’s picture

Yes it should be under 'other', I didn't find that info until after publishing this module.

NancyDru’s picture

No big deal. I would just as soon see it under "Development" because it's more for developers, but it is your module.

danielb’s picture

Status: Needs work » Closed (fixed)

I've fixed up a couple things I found as a result of #4, but there are a couple things that didn't replicate for me as well and may be related to other code around the example code. I think though those sorts of matters will have to be investigated case by case in their own issues as they are quite complex problems that may require discussion and research.

Thanks for showing some interest guys!