Domain finder

Hi,

There is already a module that makes use of phpWhois: http://drupal.org/project/whois
How does your module differ?

After installing and trying to use the module, I don't seem to get any results. Image attached. This was running from my localhost and I'm not sure if this makes any difference.

domain_finder.info
Consider adding configure = admin/config/search/domain-finder to the .info file so it is easy to find the configuration page.

domain_finder.domains.inc
Is there a better way of getting the domains that phpwhois supports so when phpwhois adds more support this file does not have to be updated?

domain_finder.admin.inc
Do you really need a submit function for your settings form? Can you not just return the results from the system_settings_form function in your domain_finder_generate_admin_form function ?

domain_finder.module
Maybe consider using the libraries module: http://drupal.org/project/libraries instead of:

$file = DRUPAL_ROOT . '/sites/all/libraries/phpwhois/whois.main.php';

You might consider implementing a hook_uninstall function in a .install file to remove the variables that your module sets.

Other than the things pointed out above, the code and comments look pretty good to me.

Hi 2pha,

thanks for reviewed our module.

This module has other goal as whois. This module just check that domain is available or unavailable in admin form preset domain extensions.

No results.
Repaired that if not just name given find probably what name want to checking. Before first use must set the domain extensions what you want to checking in admin form.

domain_finder.info -> OK

domain_finder.domains.inc
I don't know. If I created this project this was enough me, but if this project will be public maybe need other support about this.

domain_finder.admin.inc
Did not found a way to create needed variables with system_settings_form.

domain_finder.module
Yes are you right, must learn the libraries api that change this function to libraries handling.

Thanks again!
Regards,
Joe

when you return the results from system_settings_form in your admin function the saving of your variables is handled automatically as outlined at: http://drupal.org/node/1111260
So there is usually no need for a submit function for your settings form.
But you will still need to remove those variables in a hook_uninstall.

Yes, thanks!
Some change and works fine with system_settings_form :) On this variant must using "checkboxes" and not "checkbox".

Yes variables removing was implemented in last change, forgot to write here.

Now use libraries to check phpwhois library.

Please add all reviews that you did to the issue summary, as outlined in http://drupal.org/node/1410826

who is the applicant that will get the git vetted user role in this application? The sandbox belongs to dj1999 but the application was created by suhani.jain?

manual review:

1. domain_finder_domains_ext(): use arry_filter() instead of the foreach loop.
2. "if (function_exists("libraries_get_path")) {": since your are depending on the libraries module anyway this function will always be available. Remove that check.
3. "$error = t('No phpWhois library found in !path folder.', array('!path' => $path));": you should use the "@" or "%" placeholder per default if you do not want to use HTML tags in the replacement anyway.
4. "drupal_set_message(check_plain($error), 'error');": no user supplied text is involved here, so the check_plain() is not necessary. Please read http://drupal.org/node/28984 again.
5. "watchdog('domain_finder', $error, NULL, WATCHDOG_ERROR);": do not pass already translated variables to watchdog(), as the translation system will be invoked when the log messages are read.
6. domain_finder_preprocess_domain_finder_result(): $result['regrinfo']['domain']['name'] is provided by a third party service and is therefore considered untrusted user supplied input. You should sanitize that before providing it in theme variables.
7. domain_finder_get_base_url(): that function is never used, or am I missing something?
8. "module_load_include('inc', 'domain_finder', 'includes/domain_finder.domains');": no need to use module_load_include as you are including files from your own module which you already know where they are. Use something like require_once 'mymodule.inc';

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

Hi klausi,

tanks for review!

Who is? I wrote basic code suhani.jain is co-maintainer and in this case help me properly project creating.

Reviews all was changed. I do not know why easier the include_once + helper function instead module_load_include, but if it is easier than changed :)

Thanks again for reviewing! It was very useful.

Regards,
Joe

manual review:
"t($error, array('%path' => $path)": do not pass variables to t() where possible as that cannot be detected by translation string extraction tools.

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

I guess it is ok to give you both the git vetted user role once this gets approved.

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

Thanks for your contribution, suhani.jain and dj1999!

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.

Thanks for reviews and help klausi!

Regards,
Joe

moved to issues

This application is closed. Please report all issues about the module to its issue queue.