[D7] Saudi Identity Field

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.

Automated Review
PASS Automated 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
No: Does not follow the guidelines for in-project documentation and/or the README Template.
Introduction and Requirements sections missing (required).
Please see: https://www.drupal.org/node/2181737

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

- All your code is indented by two spaces (particularly clear in the .install file, where the return is on the same line of the function keyword)
- No tests provided
- Minor spelling mistake in the info file and README.md: double commas, "Saudi or none-Saudi" (should be: non-Saudi)
- Your implementation of hook_field_is_empty() is 4 lines of code, but can be optimised in just one line: return empty($item['saudi_identity']);
- In your saudi_identity_field_field_widget_settings_form(), you're assigning 2 variables, but the only usage is to retrieve a default value for your form element. Can't you just get the value there straight from the $instance array, and spare the allocation of 2 other variables?
- In your saudi_identity_field_field_widget_form(), since it's just a textfield, why can't you simply pass a normal form element? You're nesting it inside the "base" element now. I'm sure I'm missing something. Also, the wrapper markup is not really needed there (unless you're planning on some further expansion?)
- In your saudi_identity_field_valid(), there are two commas in the comment.
- The sentence "Saudi ID must be exactly in 10 numbers" could be changed to "Saudi ID numbers must be exactly 10 digits long"
- Also, you're giving credit to a 3rd party for their algorithm. Might be perfectly fine there, although I would have put it in the README.md
- There are 3 if statements in sequence. Not a great fan of that, but it's probably down to personal taste. I would create and array with the 2 types-numbers as keys (1 and 2) and the names as values, then check if $type is not a key of that array or if the value associated to that key matches $id_type, excluding "none". Ends up being 2 lines shorter and only one IF check, instead of three.

This review uses the Project Application Review Template.

Assigning it back to you for that README.md issue.

1- No probs. I always thought that "function" keyword has to start at the very far left, not indented (most of the code I've seen follows that rule, but can't find an indication in the Coding Standards about that either). But at the same time, your "return" has to be indented as well (it's not, at the moment. See line 11 of .install file). Won't argue with the Automated review tool though (since it passed).
3- Mine was just a pedantic observation :) In english, should be "non-Saudi", instead of "none-Saudi"
4- Yeah, that's correct, it's core code, but 1- returning the condition directly is still more efficient (core code sometimes has its own idiosyncrasies...) 2- if you're not doing any custom logic, probably you won't need that hook there either
5- From the documentation: Field widget form elements should be based on the passed-in $element, which contains the base form element properties derived from the field configuration. I did refactor it during my review, and was working fine.
7- No probs. Again, I was being pedantic :P It's just the double commas ,, that should be only one , not the "dot"
9- Apologies, didn't see the credits in the README. Sorry dude :P

Again, all this stuff is by no means reason for not passing review :)

Looks good!

Thanks for your contribution, Essam!

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.

We are right now very occupied with all the undertaking applications and we lean toward projects with an audit reward. Kindly assistance checking on and put yourself on the high need list [3 issues], at that point we will investigate your undertaking immediately :- )

Likewise, you ought to get your companions, associates iqama renewal or other local area individuals required to survey this application. Release them through the audit agenda and post a remark that sets this issue to "needs work" (they discovered a few issues with the undertaking) or "evaluated and tried by the local area" (they found no significant imperfections).

@3ssom It looks like a rewrite of comment #2.
I cannot say if it's a new user who tries to contribute on Drupal.org, or a spammer. If it's the first case, the user didn't notice this application has been already approved.

(I am giving credit to the users who contributed to this application. I apologize for setting it back to Fixed; it's necessary to give credits.)