Superior Colors

welcome

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
Review of the master branch:

• ./superior_colors.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function MergeArrays($Arr1, $Arr2) {
  

• Run coder to check your style, some issues were found (please check the Drupal coding standards). See below.
• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See http://ventral.org/pareview/httpgitdrupalorgsandboxesbenvb1376212git.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Go and review some other project applications, so we can get back to yours sooner.

If you got any question on that please ask!

What is the difference to the default color picker (see attachement)? can you add a screenshot? ;)

@esbenvb has been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256

Don't forget to set the status to "needs review" if you want to get a review. Also, you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

Hi

there is some coding style issue in

FILE: .../superior_colors/example_theme/css/colortemplate.css
--------------------------------------------------------------------------------
FOUND 5 ERROR(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
  4 | ERROR | Expected 1 space after colon in style definition; 0 found
  8 | ERROR | Expected 1 space after colon in style definition; 0 found
  9 | ERROR | Expected 1 space after colon in style definition; 0 found
 17 | ERROR | Expected 1 space after colon in style definition; 0 found
 20 | ERROR | Expected 1 space after colon in style definition; 0 found
--------------------------------------------------------------------------------

try to fix them.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

We 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 :-)

How do I checkout your 7.x-1.x version? From below command:
git clone http://git.drupal.org/sandbox/esbenvb/1376212.git superior_colors
I got nothing but README.txt

Please read more about Moving from a master branch to a version branch.

Multiple Applications

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

Project 1: http://drupal.org/node/1873980
Project 2: http://drupal.org/node/1870738
Project 3: http://drupal.org/node/1380540

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.

hi esbenvb,

1. In superior_colors_colors_get() function, you have three different return statements:

return $keys;
return $list;
return $colors;

In order to improve code readability, why not use the same one? Or at least keep the name similar.

2. Need below lines consider the exception of not having written permission on server?

  $css2add = superior_colors_color_get_css() . "\n";
  file_save_data($css2add, 'public://superior_colors.css', FILE_EXISTS_REPLACE);
  watchdog('superior_colors', 'CSS file created');

Thanks for your contribution, esbenvb!

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.

Congratulations on having this module approved! On the chance that you are reading this, please be advised that I downloaded your module and installed it, and tried to make it work.

My feedback is for your project page. Two things:

1) I think the sentence "Look at the example in the 'example_theme' folder." should be at the top, so that users can see what a default theme should look like. For me, I tried to retro-fit an existing theme (Bartik) to use this functionality, and I struggled a bit until I look at the contents of the example_theme.

2) The example on the project page should 'match'. In the example, we add this to the theme's INFO file;

superior_colors[colors][content][content_summary_text][name]=Content Summary Text
superior_colors[colors][content][content_summary_text][value]=444444

But later in the project page, we do not reference this in the color template example.

Last comment. Because I didn't know what I was doing, I did encounter this error:

Notice: Undefined index: color_groups in superior_colors_color_groups_get() (line 150 of /home/rumali/dgd7/web/sites/all/modules/superior_colors/superior_colors.module).
Warning: Invalid argument supplied for foreach() in superior_colors_colors_form() (line 242 of /home/rumali/dgd7/web/sites/all/modules/superior_colors/superior_colors.module).

Once I looked at the example_theme, I was able to correct this error, but you might consider adding some error handling to guide the end user if these indexes are not defined.

Congrats again!