[D7] Weinre

There are some errors reported by automated review tools, did you already check them? See http://ventral.org/pareview/httpgitdrupalorgsandboxjribeiro1971064git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

I don't have experience with the particular library but few suggestions:

In weinre.module:
Line 77: Is there any specific reason to parse the .info file (even though it's statically cached) to get the config path ? As you are not looking for some other module's .info file, I think it's pretty much OK to hardcode the admin path.
Also, if the .info file's config path is admin/config/development/weinre, $href will be '/admin/config/development/weinre', which will be an invalid URL for sub folder installations.
You can replace '#markup' with something like the following using the l() function:

'#markup' => l(t('Debug with weinre'), 'admin/config/development/weinre', array('attributes' => array(
              'id' => 'weinre-logo',
              'class' => array($class),
              'title' => $title, 
              'target' => '_blank',
              ))),

Also, I think hook_page_build() would be better than hook_page_alter as you are not actually altering anything.

@Ayesh,

The reason for parsing the info file is that we didn't want to replicate the path in different places, meaning that, if it changes, we have to change it in a single place, instead of searching for every place it's replicated.

Could you please further explain the point about the path not being valid for sub folder installations? It's not clear for me.

About using hook_page_build(), thanks for pointing this out. We are going to take a look and update if there isn't any problem.

Thanks,
Ivam

weinre.module:43
'weight' => $weight++,

Why You don't use simple integer?

Great job, Jean!
I'm sorry I didn't mention these last time, but few (very) minor suggestions.

In weinre.admin.inc:

Line 33: Can we change it to use the t() function's modifiers ? (@, ! and %).
t('For information about the configuration below, please check <a href="http://people.apache.org/~pmuellr/weinre/docs/latest/Running.html" target="_blank">weinre documentation</a>.')
to t('For information about the configuration below, please check <a href="@weinre-doc-url" target="_blank">weinre documentation</a>.', array('@weinre-doc-url' => 'http://people.apache.org/~pmuellr/weinre/docs/latest/Running.html'))

This will make translation easier and persist even if you change the URL later (and safer!).

Line 56, 64, 73, 90: You have used !seconds which prevents Drupal from sanitizing the text replacement. While you are sure that this value is perfectly safe, it's not a good idea to leave it so.
%seconds should do the job very well, and it will also wrap the replacement with <em></em> tags.

Module is really great and above are very minor suggestions. Hope admins will approve your application soon :)
Good luck!

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: http://drupal.org/node/1999780

Project 2: http://drupal.org/node/1971754

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.