[D7] Addressfield - Japanese Postal Code

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 Chen

The automated review shows no issues what so ever, good Drupal code conduct!

Manual review:

You have included a very helpful README, which explains clearly how to setup and use the module. Good work. This module also seems to be unique, so no duplication.

I installed the module and followed your guidelines. I live in Japan myself and the module works like a charm. Inserting my home postal code in the form of a 2 part text box fetches my exact address. Works as advertised.

A few nitpicks:

• The inline comments in your code are quite sparse, maybe describe a bit more of what is happening.
• In your README you tell people that, if they want to load a local version of the ajaxzip3 library, they should put in in their module directory. I think this may be considered bad practice. Drupal has the sites/all/libraries specifically for this purpose. Better advice would be to install the local version there (and maybe make your module pick it up automatically if it is installed there.)

Yet, overall, I think this module is RTBC? Anyone else?

@everright.chen

Duplication

This sounds like a feature that should live in the existing Addressfield project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the Addressfield issue queue to discuss what you need; you likely just need to provide a patch. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.
If that fails for whatever reason please get back to us and set this back to "needs review".

Indeed it feels like this should be something part of Addressfield project. @everright.chen, as per @darol100 could you please raise an issue on the referred module's so the community around it can discuss whether or not this makes sense to be part of the project? Please also reference the issue here so we can keep track. Thanks!

I'm changing the status of this issue to postponed until we can get more information from the developer.

As we change our project application process in #2453587: [policy, no patch] Changes to the project application review process we are not allowed to block applications base on module duplication. We can still recommend collaboration over competition, but we should not enforce it.

I maintain addressfield.

This is not the kind of functionality we accept in the module, because it's country specific, non-trivial, and usually dependent on an external service.
This is why there's a number of modules offering country-specific address validation/lookups:
https://www.drupal.org/project/postcodeanywhere_addressfield
https://www.drupal.org/project/addressfield_nl_postcode
https://www.drupal.org/project/addressfield_viacep

So, +1 on giving everright.chen his module.

@everright.chen,

Good work, I do not see any blockers for this module. Now you need to wait until an administrator check your project. If you want to speed up the process of admin reviewing your project you would need to do 3 manual reviews.

This module pass the automatic review.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

[Yes: Meets the security requirements.

Coding style & Drupal API usage

[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

1. Just a recommendation you should added the hook_help module.

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.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

@everright.chen
I tested this module, it seems that works well!

These are my steps:
1. Enable the module.
2. Access admin/config/people/accounts/fields.
3. Create a field with type addressfield.
4. Select "Japan" in both "Available countries" and "Default country" items.
5. Tick off "Postcode check and address lookup (Japan)" and "Enable 3-4 digit format for Japanese postal code" checkboxes in "Format handlers".
6. Save.
Then you can see the "Address lookup" button when you edit user address.
It's kindly easy to use.

Thanks

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

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
• ESLint has found some issues with your code (please check the JavaScript coding standards).

js/addressfield_jp.js: line 6, col 1, Error - Missing "use strict" statement. (strict)
js/addressfield_jp.js: line 10, col 52, Error - Missing space before function parentheses. (space-before-function-paren)
js/addressfield_jp.js: line 16, col 43, Error - Missing space before function parentheses. (space-before-function-paren)
js/addressfield_jp.js: line 17, col 48, Error - Missing space before function parentheses. (space-before-function-paren)
js/addressfield_jp.js: line 19, col 6, Error - Split 'var' declarations into multiple statements. (one-var)
js/addressfield_jp.js: line 36, col 6, Error - Split 'var' declarations into multiple statements. (one-var)
js/addressfield_jp.js: line 57, col 6, Error - "AjaxZip3" used outside of binding context. (block-scoped-var)
js/addressfield_jp.js: line 57, col 6, Error - "AjaxZip3" is not defined. (no-undef)
js/addressfield_jp.js: line 58, col 6, Error - "AjaxZip3" used outside of binding context. (block-scoped-var)
js/addressfield_jp.js: line 58, col 6, Error - "AjaxZip3" is not defined. (no-undef)

10 problems

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.

manual review:

• addressfield_jp_field_attach_presave(): why is this hook implementation necessary, why do you have to copy data around? Please add a comment.

But otherwise looks good to me, so ...

Thanks for your contribution, everright!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here 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 lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

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.

Thanks to the dedicated reviewer(s) as well.