FERank

Hi there,

your README.txt is french. Think it should be english. Same for the comments in the other files.

In your install file, it would be better to use variable_del instead of sql. Where do you configure the roles? Did not find that in the code.

Hi,

 if ($rand == 0) {
      $return = 'Mesure d\'audience';
    }
    elseif ($rand == 1) {
      $return = 'Analyse d\'audience';
    }
    elseif ($rand == 2) {
      $return = 'Statistiques web';
    }
    elseif ($rand == 3) {
      $return = 'Compteur de visite';
    }
    elseif ($rand == 4) {
      $return = 'Web Analytics';
    }
    elseif ($rand == 5) {
      $return = 'Stats web';
    }
    elseif ($rand == 6) {
      $return = 'Statistique site internet';
    }

I think you should run this strings (that are used as link text) trough Drupal's t() - translation function.

Thanks for your work.

Hi

Either you forgot to check in your last changes, or I did something wrong when I got your code. I am still seeing code as above in the comment no 3,
if ($rand == 0) {
$return = 'Mesure d\'audience';
}
etc...

i.e with French instead of English strings

This is my first review so may be I made mistake, I got your code to my machine by doing:
git clone --branch 7.x-1.x DarkoKantic@git.drupal.org:sandbox/1002Studio/1519854.git ferank

Thanks

Thank you for your reviews, your forgot to change the status of those issues either to "needs work" (you found some problems with the project) or "reviewed & tested by the community" (you found no flaws).

Review of the 7.x-1.x branch:

• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  
  FILE: ...e/drupal-7/sites/all/modules/pareview_temp/test_candidate/ferank.module
  --------------------------------------------------------------------------------
  FOUND 0 ERROR(S) AND 2 WARNING(S) AFFECTING 2 LINE(S)
  --------------------------------------------------------------------------------
   77 | WARNING | Avoid backslash escaping in translatable strings when possible,
      |         | use "" quotes instead
   80 | WARNING | Avoid backslash escaping in translatable strings when possible,
      |         | use "" quotes instead
  --------------------------------------------------------------------------------
  

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.

manual review:

1. project page must be in english.
2. git commit messages must be in english.
3. what are the branches 1.1.2 and 1.1 used for? If you don't need them anymore delete them.
4. ferank.info: all user facing text must be provided in English. Same for all your other strings in your code files.

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

Automate review has passed! good work

Manual review:

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

You should use drupal_ad_js function to include your JS on the page:

http://api.drupal.org/api/drupal/includes!common.inc/function/drupal_add...

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

README.txt file needs improving little a bit

Tell user where they can find configuration options, preferably with a link to it
i.e. To configure pathauto go to pathauto admin pages:
Home » Administration » Configuration » Search and metadata » URL aliases
admin/config/search/path/patterns

Also a bit more on what is Ferank supposed to do and where to see statistics

I enabled it on my site but I don't know what I am supposed to do next, where do I see my site usages

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

I installed the module on my site: http://www.cyber-sundae.com

I went in to the configure and disabled footer by clicking "Hide FERank link in the footer ?"

However I can still see it on the pages. The "Analytics FERank" link disappears correctly but little blue box is there. (I cleared the cahes)

Also there is error in the box: Errour inscription:
see http://www.cyber-sundae.com

1) In ferank.install file:

/**
 * Uninstall.
 */

Should be:

/**
 * Implements hook_uninstall().
 */

2) In function ferank_menu(), put the description in t().

3) Consider rewriting this if-elseif in switch-case form:

$rand = rand(0, 6);
    if ($rand == 0) {
      $return = t("Audience measurement");
    }
    elseif ($rand == 1) {
      $return = t("Analytics");
    }
    elseif ($rand == 2) {
      $return = t('Web statistics');
    }
    elseif ($rand == 3) {
      $return = t('Visit counter');
    }
    elseif ($rand == 4) {
      $return = t('Web Analytics');
    }
    elseif ($rand == 5) {
      $return = t('Web stats');
    }
    elseif ($rand == 6) {
      $return = t('Statistics website');
    }

for better code readability.

I registered to the site and error has disappeared.

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

I don't think you should offer Drupal module on your site until this review is finished and you get promoted to full module, for example if there are security issues with module there could be issues with it.

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

Have you considered translating your site to English, or at least add google automatic translation button and JS.

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

on this page
admin/config/system/ferank

Change
Note: pages admins are always ignored.
to

Note: Admin's pages are always ignored.

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

I went to
http://www.cyber-sundae.com/admin/reports/ferank-maps
http://www.cyber-sundae.com/admin/reports/ferank-tracking

and report looked good, well done!

Than as per your instructions in README.txt
"To view all the statistics and manage your account, got to
'https://www.ferank.fr/client/'"

I wen't to the site, it all works but again , problem is that the site is in French, which is nice language and I speak it little bit, but problem is most of the world still does not (sorry but Yanks have won :) )
--------------------------
So module does work, good work, its just little bits and pieces to finish, keep on!

bonne journée

• In Ferank.admin.inc line 11 I found $result = db_query('SELECT * FROM {role} ORDER BY name'); but it is not used.
• 'access arguments' => array('administer site configuration'), this permission is too generic, use your own.

Thanks for your reviews, just make sure that you pick applications that did not get a review in a long time.

Review of the 7.x-1.x branch:

• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  
  FILE: ...e/drupal-7/sites/all/modules/pareview_temp/test_candidate/ferank.module
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   16 | ERROR | Whitespace found at end of line
  --------------------------------------------------------------------------------
  

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.

manual review:

1. "arg(0) != 'admin'": use path_is_admin() instead.
2. _ferank_output_js_code(): put your javascript in a dedicated *.js file and include that for beter readability and maintainability.
3. "'<a href="https://www.ferank.fr/">' . $return . '</a> FERank';": do not create link markup yourself, use l() instead.
4. "if (variable_get('ferank_track_roles', array()))": that condition will always be true, so you can remove it.

But otherwise this looks RTBC to me. Assigning to tim.plunkett as he might have time to finally approve this. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

I granted access based on klausi's RTBC. I think that's all that needs to be done? If not, let me know.

Thanks for your contribution, 1002Studio! Welcome to the community of project contributors on drupal.org.

Michelle has granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

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.