apachesolr_location

Looking good to me, I've been involved with the process of this module and it does what it is supposed to do.

Only thing I am not sure of is the dependency on location_cck (Weird name for field api support for location).

+1 from me

This is a clean and well documented module that works as advertised. The location_cck module is indeed still in the process of being updated, but that doesn't influence this module at this point.

+1

Hi there,

Seems like there are a few issues mostly related with Drupal Coding Standards. Once you clear these issues, you can run the automated review yourself again at http://ventral.org/pareview/httpgitdrupalorgsandboxphoenix1404190git

When you receive no errors, you should update the status to "needs review".

Cheers,
osman

There are still files other than README.txt in the master branch, make sure to remove them. See also step 5 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:

• ./apachesolr_location.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function apachesolr_fields_location_indexing_callback($entity, $field_name, $index_key, $field_info) {
  

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

FILE: ...al-7-pareview/sites/all/modules/pareview_temp/test_candidate/README.txt
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 5 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: ...sites/all/modules/pareview_temp/test_candidate/apachesolr_location.info
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 9 | ERROR | Files must end in a single new line character
 9 | ERROR | It's only necessary to declare files[] if they declare a class or
   |       | interface.
--------------------------------------------------------------------------------


FILE: ...tes/all/modules/pareview_temp/test_candidate/apachesolr_location.module
--------------------------------------------------------------------------------
FOUND 42 ERROR(S) AFFECTING 40 LINE(S)
--------------------------------------------------------------------------------
  11 | ERROR | Return comment must be on the next line
  21 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
  24 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
  38 | ERROR | Missing comment for param "$entity" at position 1
  39 | ERROR | Missing comment for param "$field_name" at position 2
  40 | ERROR | Missing comment for param "$index_key" at position 3
  41 | ERROR | Last parameter comment requires a blank newline after it
  41 | ERROR | Missing comment for param "$field_info" at position 4
  42 | ERROR | Return comment must be on the next line
  50 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
  58 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
  68 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
  78 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
  88 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 108 | ERROR | Last parameter comment requires a blank newline after it
 108 | ERROR | Missing comment for param "$searcher_info" at position 1
 109 | ERROR | Missing comment for @return statement
 116 | ERROR | Array indentation error, expected 4 spaces but found 6
 117 | ERROR | Array indentation error, expected 4 spaces but found 6
 118 | ERROR | Array indentation error, expected 4 spaces but found 6
 119 | ERROR | Array indentation error, expected 4 spaces but found 6
 120 | ERROR | Array indentation error, expected 4 spaces but found 6
 121 | ERROR | Array indentation error, expected 4 spaces but found 6
 125 | ERROR | Array indentation error, expected 4 spaces but found 6
 126 | ERROR | Array indentation error, expected 4 spaces but found 6
 127 | ERROR | Array indentation error, expected 4 spaces but found 6
 128 | ERROR | Array indentation error, expected 4 spaces but found 6
 129 | ERROR | Array indentation error, expected 4 spaces but found 6
 130 | ERROR | Array indentation error, expected 4 spaces but found 6
 134 | ERROR | Array indentation error, expected 4 spaces but found 6
 135 | ERROR | Array indentation error, expected 4 spaces but found 6
 136 | ERROR | Array indentation error, expected 4 spaces but found 6
 137 | ERROR | Array indentation error, expected 4 spaces but found 6
 138 | ERROR | Array indentation error, expected 4 spaces but found 6
 139 | ERROR | Array indentation error, expected 4 spaces but found 6
 143 | ERROR | Array indentation error, expected 4 spaces but found 6
 144 | ERROR | Array indentation error, expected 4 spaces but found 6
 145 | ERROR | Array indentation error, expected 4 spaces but found 6
 146 | ERROR | Array indentation error, expected 4 spaces but found 6
 147 | ERROR | Array indentation error, expected 4 spaces but found 6
 148 | ERROR | Array indentation error, expected 4 spaces but found 6
 154 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

Source: http://ventral.org/pareview - PAReview.sh online service

None of the files in your project ends in a single new line character. Basically you must have one blank line at the end of each file.

See yourself at your project repository:

http://drupalcode.org/sandbox/phoenix/1404190.git/blob/refs/heads/7.x-1....
http://drupalcode.org/sandbox/phoenix/1404190.git/blob/refs/heads/7.x-1....
http://drupalcode.org/sandbox/phoenix/1404190.git/blob/refs/heads/7.x-1....

You may need to configure your text editor, if it alters line-endings while saving the file. I don't use NetBeans, but I had similar issues with Sublime Text 2. It could be annoying until figuring out the right settings, been there.

phoenix has a long history of using Drupal. The code looks good — although apachesolr_location_fields_indexing_callback() could be refactored to stop repeating the same snippet 4 times. But that's something for future refactorings.

You should clean out the master branch, I still see files in there. You can find the instruction helpful at Moving from a master branch to a version branch.

Woah, we now have a *bonus* system? :D

Code looks good. This project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed.

As you are a community member for 8 years now, are there any patches or other contributions that you can point me to? We can of course promote this single project for you if you don't have anything else to show (but then you would not get the git vetted user role and you would have to come back to us when you need to promote something else to a full project).

Just post some links to patches or similar that you made and that show your skills. Then I don't see any problems in approving you.

Do not artificially prolong your module just to exceed the minimum required review limits, as this could lead to questions and critique when it is reviewed again.

And of course the offer still stands: we can promote this project for you in the meantime, if you have something new you can come back to us.

@klausi I really would like to start using this using this project and I would like to have support from the maintainer. It would be recommend that you grant a git account to phoenix so he can start to maintain releases.

About not granting a full git account to phoenix: The apachesolr_location module is not the module a noob would create. I think by achieving this module phoenix displays enough knowledge of drupal to be allowed to have a full git account without abusing it. The quality of code is more than enough to allow this module to be useful for the community.

About those 120 lines of code and 5 functions. If a piece of great functionality like this module can be written in less than 120 lines and 5 functions than thats fantastic. Its not how much code it contains but how much it can do.
This module is probably just the start. I m sure when this module becomes active feature requests will come and this module could get bigger very fast. Thinking about integrating with gmap to visualize the radius and all sorts of extra stuff that can make this module great.

Finally: the main criterium should be: Is this module useful? If yes than probably the guy building it will be useful too and will probably contribute more useful stuff in future. I think in this case the answer is yes so please grant the full git account.

If we promote this single project phoenix can of course create releases and has access to the git repository. A git admin can promote a project on behalf of the user without giving the git vetted user role to them. This is the standard procedure for short projects.

But I still hope that phoenix can give us pointers to other contributions, as he has been a drupal.org member for so long. Then we can give away the git vetted user role in this process and he does not have to apply again if he wants to promote another new project in the future.

Please promote this to a full project and grant Phoenix the permisisons needed to create projects.
The reason why I am pressing this is because I have to keep referring to this module as an example.

phoenix is co-maintainer of :
http://drupal.org/project/adfs_standard

phoenix helped with :
http://drupal.org/project/views_datasource

phoenix posted patches that can be found here :
http://drupal.org/node/1414196
http://drupal.org/node/1502936
http://drupal.org/node/1350488
...
(See his post log for more)

And as a witness I say that phoenix is very very active in the local Drupal user group in Belgium. Creating and organizing events is just one of them. It would be wrong not to grant him these rights.
Some examples :
http://www.drupaleuven.be
http://drupal.be

with names such as Wim Leers, Wim Mostrey, domidc and myself I think there is enough security and support that Phoenix will take care of this module and proves enough experience within the Drupal community.

Thanks in advance

Hi,

Id like to note that this modules has indeed helped me figured how to add multiple fields to apachesolr.

Thanks nick_vh for the reference to this module and thank you phoenix for the great module :)

Sorry for changing the status. My mistake.

Green lights from me for a single project promote. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects. Assigning to tim.plunkett as he might have time to finally approve this.

There is a good deal wrong with all of the documentation. See http://drupal.org/node/1354.
Leaving this for someone else.

Okay then, let's get this done.

1. Please take a moment to make your README.txt follow the guidelines for in-project documentation.
2. Does it really make sense to put your module into this package? are there other module in ? package = Search Toolkit
3. Don't end comments with // use dot's instead
4. Tim is right, there are some general things about coding standards you should work on

But this module looks good enough for me to be promoted.

Thanks for your contribution!

I've promoted this module to a full project and now your able to create releases.

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.

Thanks to the dedicated reviewer(s) as well.

New URL
Now that this experimental project has been promoted, you'll need to update the URL of your remote repository or reclone it.

git remote set-url origin phoenix@git.drupal.org:project/apachesolr_location.git

These automated tools can only give you an overview of potential issues.
There can be false positives and there can always be something overlooked.

Instead of relying on these reports I always recommend taking some time and digg into the coding style documentation.