[D7] Domain CSS

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

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.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://drupal.org/node/2056313

Project 2: https://drupal.org/node/2055397

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.

Hello,

1. Please change your git link in top of this page with:
git clone --branch 7.x-1.1 http://git.drupal.org/sandbox/brutzelspeck/2054721.git domain_css
(Permission denied with your actual git link)

2. To review this module by other dev...
To install your module with drush from clean drupal website:
- I had to add line in settings.php after $databases definition (see https://drupal.org/node/2013835)

/**
* Add the domain module setup routine.
*/
include DRUPAL_ROOT . '/sites/all/modules/contrib/domain/settings.inc';

- after that,
drush en domain_css --uri="http://example.com" -y;


3. domain_css.install
Please patch your module with attached file.

Have a good day

#6: please patch your module with attached file. (see error on attached file).

Hi,
I went through your module and found the following issues :

.module file
> line 87
It would be better for cross database access if you use db_select() instead of querying like this directly

.install file
Please write a hook_uninstall() function to delete the table (named domain_css by using db_drop()) created by this module when it is uninstalled.

[manual review]
A couple of small things

domain_css.module:42 Small spelling mistake "pretier" should be "prettier"
domain_css.module:26; domain_css.module:27 Use t()

Other than that looks good.

Forget that, the t() thing doesn't apply.

[Edit]: Sorry for the confusion. We posted at the same time!

Your git branch is a little tangled.
You should branch as 7.x-1.x not 7.x-1.1, 1.1 would be a tag.

* README is clear and the purpose of the module looks sane.

* your package should be package = Domain Access

* Given the simplicity of what it does, I'd expect that you could add that single setting to a more central domain access admin page with form_alter, rather than requiring your own form callback just for this, but that's not a blocker really.

* You run an identical query (// Query database for configuration.) by hand three times in the code. I think you should be able to abstract that to one lookup function and re-use that.

* There seems to be no validation to check that the included file exists. That would be very helpful for anyone trying to use it or figure out what paths to use.

* Overall, I have the feeling that this module is a little too simple to demonstrate full Drupal code and security best-practices.

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.

... This one is *exactly* 5 functions and 145 lines including whitespace (excluding .install) and though it's clean in itself, there is nothing outstanding in it.

Sometimes simple is good, and this seems to be the case. I'm not suggesting you make it more complex just to get over that limit! Contributions this small are often better managed as patches to the parent module - though I understand that this small use-case may not be accepted upstream immediately.
I'm on the fence about this. I won't push it back on that, but leave it open for a second opinion.

However, I do need to recommend you correct the git branch for further review.

fixed git link

Thanks for your responsiveness. I hope that another reviewer can offer a second opinion and push this through.
I think this is a worthwhile module and simple-by-design. (though doesn't - in itself - effectively demonstrate a large number of Drupalism security issues or APIs)
Open for second review...

To be honest, this is actually OK to go RTBC - though I don't mind a late sanity-check to pick up other issues I may have missed.

You don't need domain_css_uninstall() to drop your table - Drupal will take care of removing anything defined in hook_schema for you already.
The comment Implements hook_form_FORM_ID_submit(). and hook_form_FORM_ID_validate are incorrect, those hooks do not exist.
You have a few style issues listed here: http://pareview.sh/pareview/httpgitdrupalorgsandboxbrutzelspeck2054721git.
The function query_db_for_previous_configuration() should be namespaced to your module, like domain_css_query_db_for_previous_configuration().

Settings to needs work for the namespace issue, but otherwise I think this is ready to go.

----
Top Shelf Modules - Crafted, Curated, Contributed.

Thanks for your contribution, brutzelspeck!

I updated your account to let you 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 get 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.