Lookup Browser

Hi Jordan_Fei,

first of all there are quite a few issues to sort out such as indentation, whitespace and incorrect comment formats.

You can find them all here:
http://ventral.org/pareview/httpgitdrupalorgsandboxjordanfei1862848git

Here you can check source code whether it meets drupal coding standards or not, and advise you what to change in your code. You can repeat review after your commits, and can fix those errors.

manual review:

1. I have no idea what problem this module solves. Please explain on your project page according to http://drupal.org/node/997024
2. lookup_menu(): this is a hook implementation and should be documented as such, see http://drupal.org/node/1354#hookimpl . Also elsewhere.
3. lookup_element_info_alter(): why do you kick out other #pre_render callbacks? Please add a comment.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Hi,
The line to clone the repo seems incorrect as it asked me for your password.

Jordan_Fei,

There is two issues according to the Coding standards: check Pareview.

Manual review:

• You have a typo in lookup.info file looup instead of Lookup for module name and also in description.
• You sould use images folder images of course and js for javascripts. Use README.txt for documentation in lookupsample module.
• Lookup Sample module, doesn't work. Form is there but no view is defined. In test folder is some weird stuff, like txt file with views php code, some SQL file with database export.
• I think you should work on implementation of your Lookup functionality in Form API. Information about which view should be used for modal window selection should be passed in form element array. There should not be a requirement to use that custom Javascript code for implementation. Try to pass view id instead of boolean value in form element #lookup definition.
• And be aware of module name, there is already one full project with module machine name lookup!

Update the git clone command(Remove the default name).

Please add all your reviews to the issue summary. When finishing your review comment also set the issue status either to "needs work" (you found some problems with the project) or "reviewed & tested by the community" (you found no major flaws).

manual review:

1. Rename your lookupsmaple module to lookup_browser_sample for consistency.
2. lookupsample.js: this should use Drupal.behaviors, see http://drupal.org/node/756722

Otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

no objections for more than a week, so ...

Thanks for your contribution, Jordan_Fei!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

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.

Thanks to the dedicated reviewer(s) as well.