[D7] HTTP Response Headers

hi,

I'm receiving 'warning; remote HEAD refers to nonexistent ref, unable to checkout' when I tried to clone your repository.

octoplod

@vijaycs85 : You are doing awesome job on the Drupal 8 core issue queue.
For others who don't know about vijaycs85, Dries just mentioned his name in his keynote at DrupalCon Prague this year, for his contribution towards Drupal 8 core (has seen more 140+ patches through, also helps in keeping the issue queue clean, etc.)
I'd suggest we give him a privilege of working on Drupal 8 core (we need some people there badly). If it counts I can volunteer on his behalf for reviewing the projects which would help him seeing this issue through.

Enough introduction, now back to work ;-)

• You should change your git clone link to clone the non-maintainer link of the module.
  git clone http://git.drupal.org/sandbox/vijaycs85/2108205.git http_response_headers
• The automated review shows up the issues which needs to be rectified. http://pareview.sh/pareview/httpgitdrupalorgsandboxvijaycs852108205git
• I wasn't able to clone the project : Got the following error:
  

  Cloning into 'http_response_headers'...
  remote: Counting objects: 9, done.
  remote: Compressing objects: 100% (8/8), done.
  remote: Total 9 (delta 0), reused 0 (delta 0)
  Unpacking objects: 100% (9/9), done.
  Checking connectivity... done
  warning: remote HEAD refers to nonexistent ref, unable to checkout.
  

• You project page seems to be incomplete. Please have a look at Tips for a great project page to to get an idea on how a project page should be.

All the best with the module. Will stay subscribed and provide more feedback whenever I'm able to clone the project.
Changing status to needs work. Once you fix the issues, you should change the status to needs review.
Keep up the good work!

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.

Code sniffer is still not happy

FILE: ...sites/all/modules/http_response_headers/http_response_headers.drush.inc
--------------------------------------------------------------------------------
FOUND 6 ERROR(S) AFFECTING 6 LINE(S)
--------------------------------------------------------------------------------
 36 | ERROR | Multi-line array contains a single value; use single-line array
    |       | instead
 46 | ERROR | Multi-line array contains a single value; use single-line array
    |       | instead
 49 | ERROR | Multi-line array contains a single value; use single-line array
    |       | instead
 59 | ERROR | Multi-line array contains a single value; use single-line array
    |       | instead
 62 | ERROR | Multi-line array contains a single value; use single-line array
    |       | instead
 72 | ERROR | Multi-line array contains a single value; use single-line array
    |       | instead
--------------------------------------------------------------------------------

FILE: ...sites/all/modules/http_response_headers/http_response_headers.admin.inc
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 126 | ERROR | Multi-line array contains a single value; use single-line array
     |       | instead
--------------------------------------------------------------------------------

Hello,

After uninstalling the module, there are some variables that this module set that were not deleted. You will want to use hook_uninstall() and variable_del() to remove them.

UX:
It might be useful to add a configure link from the modules page: https://drupal.org/node/542202#configure
Also having a message on admin/config/system/http-response-headers that informs you that you need to set the allowed headers in settings might be helpful.

Still has some CS issues http://pareview.sh/pareview/httpgitdrupalorgsandboxvijaycs852108205git

Manual review

• It is better to add htmls in tpl files. You can use custom theme function and add a tpl for the help hook.
• you dont have to use $_GET['q']. use request_path() instead.
• You can decrease the code lines from
  

  // Compare the lowercase internal and lowercase path alias (if any).
  $page_match = drupal_match_path($path, $pages);
  if ($path != $_GET['q']) {
    return $page_match || drupal_match_path($_GET['q'], $pages);
  }
  return $page_match;

  To
  return drupal_match_path($path, $pages) || drupal_match_path($_GET['q'], $pages);

• You can replace
  $header_rules[$header] = $header_value; with drupal_add_http_header($header, $header_value); No need for double foreach loop inside init hook.

Otherwise looking fine to me.

I did a manual review of your module and I like the idea behind it. A few very minor suggestions:

There are some line in the code which are too long for good readability. You can consider breaking them into multiple lines.
Some of these are as below:
Content in hook_help I agree no tpl is not required but consider breaking the long lines into multiple lines.
File 'http_response_headers.drush.inc' line no. 59, 76, 143, 325 and 344.
File 'http_response_headers.admin.inc' line no. 27.
Some line in 'http_response_headers_ui.module'.

Other than these all looks good and a very well written module.

Not setting to needs work as its very minor but you can consider addresing them.

manual review:

• http_response_headers_ui_form_validate(): empty function, so remove it.

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 dman as he might have time to take a final look at this.

Multiple Applications

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

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

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

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.

Updated issue summary with reviews

Back to RTBC.

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

Thanks for your contribution, vijaycs85!

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.