[D7] Logo Doodle

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.

adding links to project and repo

Hi Alan

1) please update the direct git link to ( alansaviolobo -> git , logo_doodles -> logo_doodle ) :
git clone --branch 7.x-1.x git@git.drupal.org:sandbox/alansaviolobo/2100283.git logo_doodle

2) If I edit a "Logo Doodle" node and don't change the date I get an error : "There are currently 1 doodle(s) that have an overlapping duration"
The validate function finds the node we are trying to update and see this as a duplicate..
add nid to $input_values and add "->entityCondition('entity_id', $input_values['nid'], '!=')"

3) The two conditions :
->fieldCondition('field_logo_doodle_duration', 'value', array($input_values['value'], $input_values['value2']), 'BETWEEN')
->fieldCondition('field_logo_doodle_duration', 'value2', array($input_values['value'], $input_values['value2']), 'BETWEEN')
.. are AND'et, thus overlapping doodles are only found if both dates are within other doodle ranges.
The query should OR these two conditions, see http://www.phase2technology.com/blog/or-queries-with-entityfieldquery/

4) Date's are saved in timezone UTC, but when you check the dates in logo_doodle_get_site_active_doodle() you use "date()"
which return a localized timestamp ( http://www.php.net/manual/en/function.date.php )
Use gmdate() instead ( http://www.php.net/manual/en/function.gmdate.php )
.. maybe you have the same issue with birthdays..

5) Your content_type defines an interval of 15 minutes. On a fresh install with only the dependant "date" module installed as
configured in the .info file there is no way the user can see why the dates are changed to 0/15/30/45 when saved.

added links to manual reviews of other modules

Hello,
I tried the module, and I have some feedback for it. It looks fine. But I am missing some image preset functionality. Also when there is no birthday field i got this warning:

Warning: current() expects parameter 1 to be array, boolean given in logo_doodle_birthdays_get_user_birthday_doodle() (line 81 of /home/pity/workspace/drupal_tmp/sites/all/modules/custom/logo_doodles/logo_doodle.module).

When the field is there and set up everything seem to work ok. But there is still possibility to remove the field, which will cause the same behavior with warning.

Maybe consider to make the birthday functionality as submodule with dependency to birthday module, which would also help to install the module using drush, which is able to download all missing modules from dependency.

But the module seems to work.

My guess is he means image styles. I actually thought the same, as this module open up for having a range of site logos. Normally the logo is very static and it's not a problem to scale it manually. So I will also suggest you add a image style to the image and make the w/h configurable.

Thelmer is right, this would be really nice feature. And the part with submodule is not necessary to implement, that was just for consideration. If you think the module would be too small you can leave it in one module. Confirming that the notice is fixed.

I ran this through Coder module again and it noted the following:

logo_doodle.admin.inc

severity: minorreview: style_trailing_spacesLine 56: There should be no trailing spaces [style_trailing_spaces]
*

Pretty minor, but just though I mention it in case you have a chance to fix.

I've installed the module and configured it. Will provide some feedback after testing it a bit.

manual review:

1. "The following functionality is valid only if the birthdays module is enabled.": and after that there is no code?
2. logo_doodle_menu(): 'administer logo doodles' that permission does not exist, did you forget to implement hook_permission()?
3. logo_doodle_file_url_alter(): I don't understand why that even works. Looks like every file URL on your site is changed then? How do you make sure that you only change the logo URL?

But otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to cweagans as he might have time to take a final look at this.

no objections for more than a week, so ...

Thanks for your contribution, alansaviolobo!

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.