Website Hider

Your module has some coding style issues (See http://ventral.org/pareview/httpgitdrupalorgsandboxcrispymix1368318git, you can also use this site to re-test your self)

Your code has too feee...eeew comments, please explain your thoughts in-code so other developers don't have to re-think everything.

First of all, i think this module is quite funny :-), but you have some work to do:

Review of the 7.x-1.x branch:

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Go and review some other project applications, so we can get back to yours sooner.

You can repeat this review on http://ventral.org/pareview/httpgitdrupalorgsandboxcrispymix1368318git-7...

Manual review of the 7.x-1.x branch:
README.txt

• As PHP 5 is required for Drupal 7 the line Requires PHP 5 is obsolete

website_hider.info

• 3: The double quotes are unnecessary
• Please provide a configure = admin/config/user-interface/website_hider so that in the modules overview there is a link to the configuration page.

website_hider.module

• website_hider_block_info()
• Give your block an unique identifier (instead of using form)
• Format your associative array as found in the example on hook_block_info()
• Add t() to the title of this block (info)
• website_hider_block_view()
• Make use of $delta to switch your block, see hook_block_view()
• _website_hider_get_cache()
• I think return FALSE is cleaner return value for this case.

Other findings:

• I think drupal_http_request() is a better way to do the request.
• Please check your return values, use FALSE instead of "" or NULL
• Review your code for http://drupal.org/coding-standards#linelength
• Review your code for http://drupal.org/node/1354#inline (you are using a lot of /* */ for inline comments on most of the time 2 lines.

Hi,
There is a small notice when trying to save the admin settings:
Notice: Undefined property: stdClass::$data in _website_hider_fetch_url() (line 250 of .../drupal7/sites/all/modules/website_hider/website_hider.module).
It's probably because I'm behind a proxy ...
I have just one question, I didn't understad what the difference is between this module and the maintenance mode ... besides the redirect.

Oh wow, at first I thought, that must be an easy module, but you made real science out of it! :D

• you should define your constants in global context, not in hook_init/boot or else
• it makes no sense to include your .admin.inc everytime, then define the menu path in a custom function and tell the menu system to load admin.inc again when visiting the admin page.
  All your hook_menu stuff should be contained in .module, don't include your admin.inc in global context if it is only needed on the admin page, and as you set the file argument of your menu path to the admin.inc, it will be loaded automatically.
• I don't think there is a need to create a custom permission to administer your module, checking whether the user can acces admin pages should be enough

•     '#default_value' =>
      variable_get('website_hider_url', WEBSITE_HIDER_DEFAULT_URL),

  this looks kind of ugly, btw: you only have to make sure that comments are not longer then 80 chars, code is okay!
  if you really want to put this on a new line, indent it with 2 spaces. (you should generally do more indentation within arrays)

• 'cache' => DRUPAL_NO_CACHE, is it really necessary to re-build the block on every page request? if yes, you should waive the tpl file and use a theme function instead.. (to be at least a little more performant)
• Instead of doing this init_js stuff you should put those things into an #attach key of your block
• Your module did only fetch one picture of the site I've tested it with, the rest didn't exist. Also the cached images had a strange path sites/default/files/website_hider_cache/sites/default/files
  looks like there's something wrong?
• // Regenerate the cached page file every day, every day? actually it's regenerated on every cron-run ?

• function website_hider_admin_settings_form_submit($form, $form_state) {
    system_settings_form_submit($form, $form_state);
    $website_hider_redirect
      = variable_get('website_hider_redirect', 'cache_and_redirect');

  the website_hider_redirect value is still in $form_state, saving the settings by system_settings_form_submit and then getting settings with variable get .. seems unnecessary.

• print l($button_text, $redirect_to_url); seriously, a tpl file is really not necessary IMHO
• move website_hider_theme() into your .module
• you should validate the submitted url with valid_url()
• your indentation could really be better in several places!

so long, I'll stop here and let you do some improvements ;)

you can raise the priority up again after another 2 weeks without response.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.