CVS edit link for tregismoreira

Hello,

This is the second time I send a request to this module. I hope everything goes well this time.

Before talking about the module, I would like to introduce myself: I work with Drupal development for about 2 years and have always been very motivated to help and contribute with other users. I am part of a local community in Brazil (drupal-br.org), and now I act as moderator of the forum, along with some friends. There are about 6 months, I have been devoted to the study of development of modules and got carried away with it. I have developed some modules for specific needs and I am sending this is the first completed wanting to contribute. I tried to follow all recommendations, not to run the risk of being rejected.

In the previous request, I have not explained the reason for the module and I believe that was why I was not accepted. Well, now I will explain the module:

The module is simple but very useful. It is an integration with the service Sikbox (sikbix.com) that provides the User a live search tool to the site. On the project there are some customizations to the search box is at the style of the site. All these settings have been ported to Drupal forms. That is, the customization can be made directly in the administrative page of the module, the module creates a block of search, as configured.

The request is made using the service BOSS (http://developer.yahoo.com/search/boss/), Yahoo! and the result is returned immediately. I have one I test my site in the right sidebar: http://sikbox.thiagoregis.com/ (User: admin | Password: admin). Accessing the administration area, go to Administer> Settings> Sikbox Live Search and see the settings available. By saving them, the search will return immediately to your settings.

Well, as I said, this is the first module I want to publish on Drupal.org. I am very excited about the situation and would love it to be accepted.

I have not found the field to upload the module, so I'm putting the link to download: http://sikbox.thiagoregis.com/sikbox.tar

I hope everything goes well.

Thank you!
Thiago Régis

CommentFileSizeAuthor
#5 sikbox-6.x-1.0.zip8.38 KBtregismoreira
#1 sikbox.zip8.09 KBtregismoreira

Comments

tregismoreira’s picture

tregismoreira commented
Assigned: Unassigned » tregismoreira
StatusFileSize
new sikbox.zip8.09 KB

First a little brief introduction about myself. I'm the maintainer of the Brazilian Drupal community webiste and a very enthusiastic Drupal user and "evangelist". I'm a experienced drupal developer.

Since I started working with Drupal, I wish to cooperate in any way with the community. I currently collaborate with Support and Translation. I've created several Drupal modules but now I have one that I want to give back to the community. This modules is called Sikbox and it's a 3rd part integration, with the Sikbox service (http://sikbox.com).

Sikbox is a service that allows you to add a full featured live search to your site or blog within minutes. It's implements BOSS (http://developer.yahoo.com/search/boss/), from Yahoo!, to provide a customized live search widget.

It's necessary setting just 3 options to get your search widget: URL (optional), Number of results and Skin. You can see this module in action at http://sikbox.thiagoregis.com (sidebar right).

You can download the code at http://sikbox.thiagoregis.com/sikbox.zip.

Thanks!
Thiago Régis
@tregismoreira
http://thiagoregis.com

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: tregismoreira » Unassigned
Status: Postponed (maintainer needs more info) » Needs work
  1. The module doesn't have the installation file, which should be used to remove the Drupal variables used by the module while uninstalling it. 
  2. /**
 * Implementation of hook_submit().
 */
function sikbox_settings_submit($form, &$form_state){
  variable_set('sikbox_url', $form_state['values']['sikbox_url']);
  variable_set('sikbox_results', $form_state['values']['sikbox_results']);
  variable_set('sikbox_skin', $form_state['values']['sikbox_skin']);
}


/**
 * Implementation of hook_validate().
 */
function sikbox_settings_validate($form, &$form_state) {
  if (!empty($form_state['values']['sikbox_url']) && !_sikbox_validate_url($form_state['values']['sikbox_url'])) {
    form_set_error('url', t("The domain name <em>'@url'</em> is not valid.", array('@url' => $form_state['values']['sikbox_url'])));
  }
}

    Those functions are not implementations of hooks; the submission function is useless, as the variables are already saved from system_settings_form_submit().

  3. Strings used in the user interface should be in sentence case.
tregismoreira’s picture

tregismoreira commented

Ok, thanks. I'll make the necessary modifications.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Closed (won't fix)
tregismoreira’s picture

tregismoreira commented
StatusFileSize
new sikbox-6.x-1.0.zip8.38 KB

I fixed the missing details. Please evaluate again.

Thanks!
Thiago Régis
@tregismoreira
www.thiagoregis.com

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Closed (won't fix) » Needs review
tregismoreira’s picture

tregismoreira commented

?

tregismoreira’s picture

tregismoreira commented

I need to know if you've evaluated the module. I need that.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: Unassigned » avpaderno
Status: Needs review » Needs work
  • The points reported in this review are not in order or importance / relevance.
  • Most of the times I report the code that present an issue. In such cases, the same error can be present in other parts of the code; the fact I don't report the same issue more than once doesn't mean the same issue is not present in different places.
  • Not all the reported points are application blockers; some of the points I report are simple suggestions to who applies for a CVS account. For a list of what is considered a blocker for the application approval, see CVS applications review, what to expect. Keep in mind the list is still under construction, and can be changed to adapt it to what has been found out during code review, or to make the list clearer to who applies for a CVS account.
  1. License files cannot be committed in Drupal.org repository. Projects committed in Drupal.org repository have the same license used by Drupal.
  2. Menu descriptions and titles, schema descriptions are not passed to t().
  3. Hook implementation comments should be like the following one: 
    /**
 * Implements hook_menu().
 */
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Closed (won't fix)