[D8] Sodium Encryption

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. Longest one I've seen all day actually. So many people creating an empty one and not bothering with it. nice to see you trying.

Code long/complex enough for review

[No: Does not follow] the guidelines for project length and complexity. Less than 5 functions AND less than 120 lines. This is way to short. Are there plans to extend it?

This review uses the Project Application Review Template.

I don't think its officially in the guidelines but I did see a discussion leaning towards granting the permission to promote based of contributions outside the module in the application.

The idea being that a short module is insufficient to show you know the drupal coding style etc well but if you can show you know it in other ways then the permission to promote can be granted.

It might be worth linking a couple of your commits from those projects so even though this project is insufficent to grant you the ability to promote it sounds like you do have the experience necessary.

Just took a look at your profile. I am impressed by the number of commits. I really hope you do know what your doing or theres gonna be a lot of dodgey encryption modules out there.

Maybe you should ask the co-maintainers of those projects to swing by and offer their opinion on you rcompetence with drupal coding.

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

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.

I'll say that in the past few years rlhawk and I have worked on several issues and he's provided quite a lot of high quality code and reasonable perspectives.

One thought about the length/function number requirement, perhaps you could create some automated tests for this code? That would help to satisfy the letter of the rule and also improve the project.

In general review of the module, I'm not sure it makes sense to catch exceptions and turn them into messages. The code calling this method may want to display its own appropriate message or do something else that makes sense in the context (e.g. drush_log or retry encrypting/decrypting in a different way). Perhaps a more complete solution would be for the encrypt.module to define a set of Exceptions that can be thrown from the encrypt/decrypt methods. Then the responsibility of this Sodium encryption module would be to map the HaliteAlerts into the appropriate encrypt.module exceptions?

Hi @rlhawk,

Code too short

This project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project for you.

@rlhawk in #14 thanks for adding the tests. Perhaps while this is getting reviewed you'll find more time to add some more structure around exceptions?

@th_tushar - thanks for reviewing the project. I used the sloccount tool and it finds 110 lines of php code, so nearly at the 120 limit. A simple grep for function declarations finds 6. As the discussion you linked shows the "120 lines/5 functions" is a guideline and the history of contributions should be factored in for those cases. If you look at rlhawk's profile you can see he's done a ton of other work which I think justifies granting git vetted user status rather than doing the single project promotion.

Ok. Removing the Single project promote tag. :-)

@th_tushar Considering though that we are vetting the individual for git vetted status surely its reasonable to go outside one project in some cases. In this case checking rlhawk's profile shows the project he has contributed to the most as Key. Hes also the largest commiter of that project in terms of number of commits by a considerable margin

@rlhawk I've seen plenty of debate regarding giving git vetted status to people who show they understand the Drupal way of doing things but only have a short module to submit. Its up to yo uhow much hassle you want to go through. You could go for single project promotion or you could hope that the first git overlord to look into the issue decides your other work is good enough to grant you access.

Assuming you did want to go argue for git vetted status based on your other projects I'd start by doing an automated review of the Key module and sort the issues there. I've just done one and Key is far from best practice.
http://pareview.sh/pareview/httpgitdrupalorgprojectkeygit

Either way why not take this as an opportunity to do some sprinng cleaning and go fix the automated test errors on Key. Too many modules let the standards slip once they are released.

@greggles While I've seen people on both sides of the debate and much debate on it, personally I feel any user has commited to Drupal in such a huge way deserves git vetted status, assuming their code is up to scratch.

Looks good to me after a manual review. Since greggles is another git admin and has also reviewed this already I think we can approve this right now.

Thanks for your contribution, Rick!

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.