Code review is needed for NuoDB Drupal V7 Database Driver. This driver allows NuoDB to be used as the back end database for Drupal 7.x.
Project Page: https://drupal.org/sandbox/tgates/2191439
Git: git clone --branch 7.x-1.x git.drupal.org:sandbox/tgates/2191439.git d7_nuodb_driver
Requires a NuoDB installation and NuoDB PHP PDO setup: http://www.nuodb.com
Driver installation and configuration instructions are in the README.txt file.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | nuodb_driver_codesniffer_results.txt | 3.74 KB | rob.barnett |
Comments
Comment #1
tgates commentedComment #2
PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxtgates2191439git
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.
Comment #3
PA robot commentedClosing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #4
tgates commentedComment #5
tgates commentedI fixed most of the issues flagged by the automated review tools. However, the automated review tools is wrong for database driver modules. The Drupal automated code review wants the class names to start with the project name "NuoDB". But that is inconsistent with the Drupal v7 driver code, which expects Drupal 7 driver modules to have class names appended with underscore "_" and the driver name "nuodb". (ex: DatabaseConnection_nuodb).
-tom
Comment #6
tgates commentedComment #7
znaeff commentedHi.
1. Please remove master branch and create new branch (like '7.x-1.x') then delete master branch.
2. Fix git link in issue summary.
Change
git clone --branch master tgates@git.drupal.org:sandbox/tgates/2191439.git _d7__nuodb_driver
to
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/tgates/2191439.git _d7__nuodb_driver
3. Also I'm suggesting to create nuodb.install file with requirements existing checking.
Comment #8
tgates commentedComment #9
tgates commentedThanks Znaeff. I made the changes you mentioned in the previous comment.
-tom
Comment #10
tgates commentedComment #11
rob.barnett commentedAutomated Review:
Code Sniffer still shows errors would should be addressed. These include issues found in your .install not related to your response in comment #5.
Manual Review:
In .install you should call a link with the l function or url function
I would suggest adding a hook_help in your .module and add the contents of your README.txt. This is nice for site builders.
Otherwise it looks good.
Comment #12
tgates commentedI have made all of the changes from the last code review and re-ran Code Sniffer on the latest code. All of the outstanding issues that pertain to a Database driver module has been resolved.
Comment #13
tgates commentedComment #14
kscheirerPlease don't RTBC your own issues. If you wish to escalate your issue, please follow the procedure detailed here: https://www.drupal.org/node/539608#application-priorities
Comment #15
tgates commentedComment #16
tgates commentedThis has been waiting three months for a review. Is that normal? What's the holdup?
Comment #17
klausiWe are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)
Comment #18
andrefy commentedCould clarify the git source I got some authentication issues while cloning git clone --branch 7.x-1.x tgates@git.drupal.org:sandbox/tgates/2191439.git _d7__nuodb_driver
I would recommend to use this https://www.drupal.org/node/2190239 template, that will make it easier to review, thanks
Comment #19
andrefy commentedComment #20
tgates commentedComment #21
tgates commentedFix link to git repo. Updated project page.
Comment #22
fabianx commentedGuidelines followed, found no security problems. (I know some of the mysql driver code and necessary escaping is in place)
=> RTBC
Comment #23
cweagansThanks for your contribution!
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:
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.