Description:

This module provides a "bridge" between the message module and the jGrowl notification functionality provided by the nodeJS integration module. This allows customizable "real time" growl like notifications that fire when message events occur.

Sandbox:

http://drupal.org/sandbox/rho/1496824

Git:

git clone --recursive --branch 7.x-1.x rho@git.drupal.org:sandbox/rho/1496824.git messagejs

Version:

This module is intended for D7 only.

Comments

patrickd’s picture

patrickd commented

welcome,

good to see you've already fixed the coding style issues.

As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow the tips for a great project page.

We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

regards

rho_’s picture

rho_ commented

Thanks for the welcome patrickd!

I've updated the project page per your suggestion, including installation instructions and descriptions/visualizations of usage.

Thank you, and all of the reviewers for what you do. I will jump on this evening and try and knock a few reviews out. Is it sufficient to post the reviewed projects here in the comment thread to obtain the "review bonus?"

patrickd’s picture

patrickd commented

Thank you, we really appreciate all help we can get. As you can still edit the issue summary just add the list of reviews (direct link to the comment) into it and add a review bonus tag (PAReview: review bonus) by a new comment.

Cristian.Andrei’s picture

Cristian.Andrei commented

Please have your code reviewed by ventral.org/pareview and fix all issues. Right now it states that it
FOUND 6 ERROR(S) AFFECTING 6 LINE(S)

rho_’s picture

rho_ commented

Odd, it was running clean a week or so ago. Fixed the issues, seems to be running clean again.

Cristian.Andrei’s picture

Cristian.Andrei commented

Hi,

Thanks for fixing the issues. Had a look at your code. One small thing to clear might line 53 in messagejs.module :
* See message.api.php
Since this file is not included in the module, please rephrase that so that it is clear that the message.api.php is found in the Message module.

Otherwise, it looks nice & clean. Also, tested the module and it works fine.

rho_’s picture

rho_ commented

Thanks Cristian!

I've changed "See message.api.php" to "See message.api.php found in the message module folder." Changes have been pushed.

eyal shalev’s picture

eyal shalev
he/him
commented

Hi,

Can you change your default branch to 7.x-1.x (currently master).

And also edit this issue description to include the new git clone command (will be helpful to new reviewers).

rho_’s picture

rho_ commented

7.x-1.x is now set as the default branch, and the issue description here has been updated.

eyal shalev’s picture

eyal shalev
he/him
commented

Awsome :)

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

this looks really good to me.

- in the README, change 'mysite.com' to 'example.com'.

- i don't think messagejs_nodejs_handlers_info() is necessary, as you have nodejs_notify as a dependency.

i don't think either of these should hold up this becoming a full project, RTBC.

rho_’s picture

rho_ commented

It didn't occur to me, but you are absolutely correct. hook_nodejs_handlers_info is redundant in this module as nodejs_notify does it already.

Removed the function, and made the change to the readme.

Thanks beejeebus!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Reviewed & tested by the community » Fixed

Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updating repository information.