Allows access to data provided by the 51Degrees.mobi device detection service which provides intelligence about device, operating system and browser capabilities of visitors to your site. This allows developers and designers to adapt the response from the server and provide a customised browsing experience and allows existing websites to quickly support the growing number of mobile browsing devices.

This modules is intended for Drupal 6.x

Sandbox page : http://drupal.org/sandbox/51Degrees/1617942

Documentation : http://51degrees.mobi/Support/Documentation/PHP.aspx

Repository : http://git.drupal.org/sandbox/51Degrees/1617942.git

CommentFileSizeAuthor
#6 Drupal6-51Degrees.mobi-Lite-2012.06.20.php_.zip1.99 MB51Degrees.mobi
Drupal651DegreesmobiLite20120608.zip2.35 MB51Degrees.mobi

Comments

ordermind’s picture

ordermind commented
Status: Needs work » Needs review

Manual Review
1. Take a look at http://drupal.org/node/206753 at make sure you follow the standards there. The module name must consist of only lowercase letters and underscore, and it should be used as a prefix for all the functions that are defined by the module. For example, if you call the module "mobile_device_detection" the .module file should be called "mobile_device_detection.module" and all the functions there should carry that prefix. The .info file for the module should in that case be called "mobile_device_detection.info".

2. When defining tables, use hook_schema together with hook_install and hook_uninstall in a .install file rather than the include.sql.txt you have now.

3. Make sure you follow the coding standards detailed at http://drupal.org/coding-standards, there are many such errors in the module.

ordermind’s picture

ordermind commented
Status: Needs review » Needs work
patrickd’s picture

patrickd commented
Status: Needs review » Needs work

You should point out how to detect those coding standard errors you mention.

Either use drupalcs (code sniffer) or ventral.org/pareview

ordermind’s picture

ordermind commented

Right, sorry. You can also use Drupal Coder - http://drupal.org/project/coder to detect coding standard errors.

51Degrees.mobi’s picture

51Degrees.mobi commented
Project: Drupal.org security advisory coverage applications »
Component: module » Code
Status: Needs work » Needs review

Thank you for you help in reviewing this module.

We have worked hard to get the code up to standard and have been using ventral.org/pareview as our guide. I believe that we are very close to having the module ready to go but would like to ask for your view on a couple of things.

We use csv file to reference our devices. These files have not been renamed with our module names as they are accessed often and the names were designed for the purpose of ensuring speed within the module. They are also all located within their own subdirectory. would it be possible to not have these renamed or it is essential that they are before the module would be accepted?

I have attached a zip file with the project for review. All help is appreciated.

Phil

51Degrees.mobi’s picture

51Degrees.mobi commented
StatusFileSize
new Drupal6-51Degrees.mobi-Lite-2012.06.20.php_.zip1.99 MB

Project zip file attached here

ordermind’s picture

ordermind commented
Status: Needs review » Needs work

I believe that the names for the csv files can stay the way they are as there is a good reason for it and there shouldn't be any risk for compability issues. However, there are still other problems in the module that need to be resolved:

- Please create a 6.x-1.x branch and get rid of the master branch, see http://drupal.org/node/1015226 for info.

- You still use the fiftyone_degrees_include_sql.txt file instead of a .install file, please read http://drupal.org/node/51220 for more information about how to use installation hooks.

- It seems as though the module should really be called "fiftyone_degrees" instead of "fiftyone_degrees_include" since you use it as prefix for the other files in the root directory. Please make that change and move the other files to a ./include directory so that they don't clutter up the root directory.

- The functions in fiftyone_degrees_include.module all need to carry the prefix of the name of the module, see http://drupal.org/coding-standards/#naming. For example, if your module is called "fiftyone_degrees.module" then all functions in it (as well as all include files) should carry the prefix "fiftyone_degrees_".

- Please make sure that your module passes the automatic testing of code standards at http://ventral.org/pareview. Here's a list of errors that were found at this time: http://ventral.org/pareview/httpgitdrupalorgsandbox51degrees1617942git

51Degrees.mobi’s picture

51Degrees.mobi commented
Status: Needs work » Needs review

Right,
I think we are there.

* The new branch was created to house the project.
* The correct .install file added to the project.
* Include directory added to ensure root is kept clean.
*Releavant files renamed to match the function/module name.

The code has been passed through ventral.org/pareview through the development process and I hope that the module is now ready.

The existing errors that show up in the testing relate to the automatically generated files that were mentioned in the previous post. There also exists some minor warnings that we will aim to look at within our overall project files.

The project can now be found at http://git.drupal.org/sandbox/51Degrees/1617942.git 6.x-1.x
Thanks to all for your help

Phil.

51Degrees.mobi’s picture

51Degrees.mobi commented
Assigned: 51Degrees.mobi » Unassigned

This may not have been assigned correctly.

------------------------------------------------

Right, I think we are there.

* The new branch was created to house the project.
* The correct .install file added to the project.
* Include directory added to ensure root is kept clean.
* Releavant files renamed to match the function/module name.

The code has been passed through ventral.org/pareview through the development process and I hope that the module is now ready.

The existing errors that show up in the testing relate to the automatically generated files that were mentioned in the previous post. There also exists some minor warnings that we will aim to look at within our overall project files.

The project can now be found at http://git.drupal.org/sandbox/51Degrees/1617942.git 6.x-1.x

Thanks to all for your help

Phil.

51Degrees.mobi’s picture

51Degrees.mobi commented
Assigned: Unassigned » 51Degrees.mobi
Status: Needs review » Postponed

Hi,

We're currently working on a big update for the plugin, featuring a much more comprehensive UI and a theme switching system that should be ready later this week. Would you be available to review it?

Tom

51Degrees.mobi’s picture

51Degrees.mobi commented
Status: Postponed » Closed (won't fix)
51Degrees.mobi’s picture

51Degrees.mobi commented
Issue summary: View changes

Added repository info

Project: » Lost & found issues

This issue’s project has disappeared. Most likely, it was a sandbox project, which can be deleted by its maintainer. See the Lost & found issues project page for more details. (The missing project ID was 1617942)