Localization server project sets

Hello,

I can't find L10n_community to install - http://drupal.org/search/apachesolr_multisitesearch/L10n_community ?

Is it http://drupal.org/project/l10n_server ? Or - http://drupal.org/project/l10n_install?

Maybe add these to the readme.txt

Thanks

Added second project review.

I thought it might be something too technical for me, but I always try to offer as much help as possible so people don't get stuck :)

@lucascaro: don't forget to add the review bonus tag if you did reviews of other projects as outlined in #1410826: [META] Review bonus.

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

• Run coder to check your style, some issues were found (please check the Drupal coding standards). See attachment.
• Drupal Code Sniffer has found some issues with your code (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.

Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards

sites/all/modules/pareview_temp/./test_candidate/tests/l10n_projectsets.test:
 +143: [minor] in most cases, replace the string function with the drupal_ equivalent string functions
 +193: [minor] in most cases, replace the string function with the drupal_ equivalent string functions

Status Messages:
 Coder found 5 projects, 5 files, 2 minor warnings, 0 warnings were flagged to be ignored

FILE: ...tes/all/modules/pareview_temp/test_candidate/l10n_projectsets.admin.inc
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 16 | ERROR | Line indented incorrectly; expected 4 spaces, found 6
 16 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
 23 | ERROR | Closing brace indented incorrectly; expected 6 spaces, found 4
--------------------------------------------------------------------------------


FILE: ...es/all/modules/pareview_temp/test_candidate/tests/l10n_projectsets.test
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 143 | ERROR | Line indented incorrectly; expected 6 spaces, found 7
--------------------------------------------------------------------------------

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

manual review of the 7.x-1.x branch:

l10n_projectsets.module
Not really important but this

function l10n_projectsets_set_load($uri) {
  $set = l10n_projectsets_get_set($uri);
  return $set;
}

can be shortened to

function l10n_projectsets_set_load($uri) {
  return l10n_projectsets_get_set($uri);
}

since $set is never used except for the return.

l10n_projectsets.admin.inc
The config page for this module is empty when nothing has been configured so far. I think it's a good idea to add some descriptive text so that find their way around (yes, I know there is a README but still).

export.inc
This is pedantic I know but there is a newline missing at line 26 (the docstring of the new function comes immediately afterwards with no newline).

forgot to change status, left review bonus tag for klausi to remove.

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

• Drupal Code Sniffer has found some issues with your code (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.

manual review:

1. Please take a moment to make your project page follow tips for a great project page.
2. l10n_projectsets_get_sets(): The first if() branch is useless where you populate $sets and then you never use it. I thnik you can remove it and just keep the else{} branch.
3. l10n_projectsets_get_sets(): indentation levels should always be 2 spaces, also for chained DB API calls. Also elsewhere.
4. l10n_projectsets_get_set() and l10n_projectsets_set_load(): why do you need both? Just keep l10n_projectsets_set_load() and move everything there?
5. l10n_projectsets_set_new(): why do you need a function for an empty object?
6. l10n_projectsets-coding_standards-1585740-9.patch: do not commit patch files into your git repository.
7. l10n_projectsets_explore_set(): don't do this title hacking. Use a title callback in hook_menu() to make the page title right. Same for l10n_projectsets_export_page().
8. You are using the "@" operator quite often to suppress warnings. In most cases that is a bad idea because it can get very hard to track down errors in your module. See also the warning on http://php.net/manual/en/language.operators.errorcontrol.php
9. l10n_projectsets_export_form_validate(): why do you need that? Just set the form element as #required => TRUE and the form API will validate that for you.
10. l10n_projectsets_export_form_submit(): don't use header(), use drupal_add_http_header() instead. Don't use die(), use drupal_exit() instead. Same in l10n_projectsets_export().
11. "watchdog('l10n_projectsets', "Added $serve_name.");": don't embed variables directly into translatable log messages, use placeholders with watchdog().

Otherwise I think this is nearly ready. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

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: ...pace/drupal-7/sites/all/modules/pareview_temp/test_candidate/export.inc
  --------------------------------------------------------------------------------
  FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   159 | WARNING | A comma should follow the last multiline array item. Found:
       |         | $serve_name
  --------------------------------------------------------------------------------
  

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:
l10n_projectsets_export_page_title(): I think the menu title callback should live in the module file: the hook_menu() docs say for the file key "Does not apply to other callbacks (only page callback)." Would be interesting to track the calling code down in Drupal core to check whether the file is included before the title callback or not. I'm a bit lazy and grepping for "title callback" did not yield any good result.

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.

Thanks for your contribution and welcome to the community of project contributors on drupal.org!! :)

I've 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.

(doubleposted by drumming on my mouse x´D)