[D7] GMap Polygon Field

Hi lingros,

Kindly see the automated review of your project as its showing some errors.

Link: https://pareview.sh/node/1948

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgsandboxlingros2877719git

Fixed the git clone URL in the issue summary for non-maintainer users.

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.

Hi @lingros,
i'm picking up this issue to start a review on it!
I'll get back to you, at most in Monday.
Have a good weekend,
Bests,
Balazs.

Hi @lingros,
as i've promised, here are the details of my review:

Automated review

Review of the 7.x-1.x branch (commit 428169d):

• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

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. You have to get a review bonus to get a review from me.

Also here is a pure phpcs output:

FILE: gmap_polygon_field.js
----------------------------------------------------------------------
FOUND 3 ERRORS AND 1 WARNING AFFECTING 4 LINES
----------------------------------------------------------------------
79 | WARNING | [x] There must be no blank line following an inline
| | comment
96 | ERROR | [x] Comments may not appear after statements
97 | ERROR | [x] Comments may not appear after statements
108 | ERROR | [x] Comments may not appear after statements
----------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Manual review

- If the default widget is 'gmap_polygon_field_widget' that one should be used in gmap_polygon_field_field_widget_info() instead of the default formatter, as e.g. file module does (references: https://api.drupal.org/api/drupal/modules%21file%21file.field.inc/functi... https://api.drupal.org/api/drupal/modules%21file%21file.field.inc/functi... - widget is file_generic) also in the widget_form where the type is checked, it should not work like this.
- In gmap_polygon_field_field_formatter_view() and gmap_polygon_field_field_widget_form() you don't need to check do the variables are empty to give values to data's array items, just use variable_get()'s second parameter with the right values like instead of:
!empty(variable_get('gmap_polygon_field_stroke_color', NULL)) ? variable_get('gmap_polygon_field_stroke_color') : "#000000"
use this:
variable_get('gmap_polygon_field_stroke_color', '#000000')
- As module implements a new field type, i advice to make it visible with a dependency in its info file to field module.

These two things are recommendations, not at all blockers, but could improve the module's quality:
- I would definitely define a permission for administration of this module instead of using the 'administer site configuration'.
- I would also write element validations for the admin form in order to ensure the inputs are the ones that the descriptions ask.

Overall i didn't feel too lost reading the module file even it has almost no inline comments, it seems pretty self-explained, but for the js i've much appreciated the well documented code!

I would say, based on the results above, it still needs some things to be done before we accept it, especially the phpcs output and the 2 first things from the manual review, the rest even can be done later in a new release :)

Bests,
Balazs.

Hi @lingros,
i've checked your fixes and it seems good, except the dependency how you fixed it - here is a patch that contains the right one.
Also please take a look on the opened issues with patches on the module's issue queue, the ones that you mentioned you have no time, got implemented by me :)
Bests,
Balazs.

Hi @lingros,
i'm glad my work helped you achieving goals :)
I've rechecked the module now and found zero issues so i don't see any reasons to not make the application RTBC!
Have a great weekend!
Bests,
Balazs.

Thank you for your contribution!
I am going to update your account so you can opt into security advisory coverage now.
These are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, 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.

I thank all the dedicated reviewers as well.