This project is module which create a custom block handler for drupal 6.x
It defines blocks from specific class and load them from predefined files with the use of the parameters.
It tries to make it easy a management of many blocks.

Project: http://drupal.org/sandbox/underq/1494508

GIT Repository details: git clone --branch 6.x-1.x http://git.drupal.org/sandbox/underq/1494508.git

Version: Drupal 6

Comments

DrupalDriven’s picture

DrupalDriven commented
Status: Needs review » Needs work

This module lack of documentation - i don't understand where i need to create files, how to name them, and what content these files must include. Please provide readme.txt with module.

Also found some issues:

  1. All strings must be in English - and you use French in alg_block.router.inc at line 62
  2. using "throw new Exception()" is not good solution - its better to use drupal_set_message() to display pretty-looking error message to admin
  3. Including module files in hook_init() is not optimal - because this hook is called on every page even if module is not perform any work. My advice - move module_load_include() to alg_block_block() so files will be included only when needed.
Jeffrey C.’s picture

Jeffrey C. commented

Automated Review:
An automated review of your project has found some issues with your code; As coding standards make sure projects are coded in a consistent style we please you to have a look at the report and try to fix them. Anyway, note that issues found are possibly false positives and fixing all issues is not a requirement for getting through the application process.

You can find the results of the automated report at http://ventral.org/pareview/httpgitdrupalorgsandboxunderq1494508git.

underq’s picture

underq commented

@Drupaldriven : I resolved issues #1 and #3 but for #2 I would prefer to keep one "throw new Exception()" in this case.

@legendm33066 : DrupalCS review done.

underq’s picture

underq commented
Status: Needs work » Needs review
hussainweb’s picture

hussainweb
Primary language English
commented
Status: Needs review » Needs work

I checked again and the README.txt is missing. I went through the code and even though I think I know how to use this module, it is not straightforward. I would recommend outlining this in README.txt.

The code looks fairly well structured. I would recommend renaming the test module to example as it seems to be an useful example. Also, it is not clear that autoload, etc... are required in the example module, or a custom module that someone would create. I would recommend outlining this in README.txt.

Few small issues:

  • alg_block.info and alg_block.interface.inc are marked as executable.
  • You are still using a French string at alg_block.router.inc:92.

Update: There are blank lines at the end of .info files. My mistake, sorry for the confusion.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Closed (won't fix)

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

If you reopen this please keep in mind that we are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)