[D7] Language Icons Admin

You have some automated testing errors:

http://pareview.sh/pareview/httpgitdrupalorgsandboxgonssal2361933git

Also, it'll be nice if you include the link to the automated coding standards validation into the issue description.

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

gonssal,

First let me thank you for your module and for contributing back to Drupal. Second, let me say that I am a first time reviewer and new to Drupal developement. Feel free to consider that along with my review.

Automated Review

The pareview tool indicates ( in addition to the items that you opted-out of fixing ) that you have lines exceeding the standard 80 characters, this seems a simple thing to remedy.

The coder_review Drupal module, has only a minor warning with four occurances:

Line 42: String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms [style_string_spacing]
$delta_element['#title'] .= ' ' . $language_icon;

So basically two spaces instead of one after the dot. Pretty nit-picky but again simple enought to fix.

Manual Review

The code is well commented and easy to follow.
The code is well formatted, "end of" comments help with code clarity.

Individual user account

Yes: https://www.drupal.org/u/gonssal.

Master Branch

Yes: Follows

Licensing

Yes: Follows

3rd party assets/code

Yes: Follows

README.txt/README.md

Yes: Follows

Code long/complex enough for review

No: Does not follow (200+ Lines of code 4 Function Calls) - However, the code does demonstrate an understanding beyond the really simple stuff.

Secure code

Yes, meets the security requirements ( Please note however I am very new to drupal. )

Coding style & Drupal API usage

1. No recommendations, there seem to be only minor variances from coding styles.

• Lines over 80 characters
• Extra space in string concatenation

This review uses the Project Application Review
Template.

I was trying to test this module, but I was unable to see anything in the backend.

A quick analysis of your sourcecode revealed the following:
File: languageicons_admin.module
Row: 75
You are trying to read 'translations'. In my test-system I only have a 'translation' key.
I have not looked any further in this so far. Maybe I need to install another module?

Testsystem:
Drupal 7.32
- Language Icons
- Language Icons Admin
- Administration Menu
I've installed two languages, English and German
Also, I've translated a few standard articles.

Tobi

Automated Review

Only minor stuff, like line indentation.

Manual Review

Individual user account

Yes

No duplication

Yes: I could not find any duplication.

Master Branch

Yes: Uses 7.x-1.x

Licensing

Yes: No licensing info was included, this will be done by the drupal server later on.

3rd party assets/code

Yes: I could not find any.

README.txt/README.md

Yes: Readme is included

Code long/complex enough for review

Yes: It is imho a little bit short, otoh it looks like that the author knows what he is doing.

Secure code

Yes: No problem that I could see. There is no real user interaction. You only use what is already inside of the system, no outside stuff.

Coding style & Drupal API usage

1. (*) I was not able to see any difference using the standard Drupal translations. Entity translations worked flawlessly.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

This review uses the Project Application Review Template.

guelzow forgot to change status to "need work" after his review apparently, so I'm changing it for him.

That review did not include any apparent application blockers. @guelzow anything else that you found or should this be RTBC instead?

@klausi
The readme file states, that it works with the standard translations.
I was not able to see any difference in the output, so I asked if I was missing something, like an other module.

I was briefly looking into the code so see what was actually happening in there:
The module tries to read values in the form array, which do not exist ('translations' key). I changed that value, but without having any impact.

Tobi

klausi, in guelzow review this says it should go into "Needs work" state:

(*) I was not able to see any difference using the standard Drupal translations. Entity translations worked flawlessly.

The starred items (*) are fairly big issues and warrant going back to Needs Work.

Automated Review

No. Today, there are errors: http://pareview.sh/pareview/httpgitdrupalorgsandboxartsakenos2122323git

Manual Review

Individual user account

Yes. You can read https://www.drupal.org/node/272587

No duplication

Why don't add your functions to language_icons module?

Master Branch

Yes. You can check: https://www.drupal.org/node/1127732

Licensing

Not, you must remove the license, you can check: https://www.drupal.org/licensing/faq

3rd party code

It's ok, too: https://www.drupal.org/node/422996

README.txt/README.md

Not, at least you must speak about the dependencies. You can check:
+ https://www.drupal.org/node/447604
+ https://www.drupal.org/node/2181737
Another tip, for improve the documentation is to implement hook_help.

Code long/complex enough for review

Not, you must write from 5 functions. You can check: https://groups.drupal.org/node/195848

Secure Code

Yes, you can check https://www.drupal.org/writing-secure-code

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.