Views GSA Feed

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

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 put yourself on the PAReview: review bonus high priority list. Then I'll take a look at your project right away :-)

Nice module Travis!
Here is my manual review:

Project Page
- Looks good!

README.txt
- I would modify your README.txt to match the following sections of your project page: Overview, Features, Requirements, Installation, Documentation, Known Problems.

Documentation
- I would put a link back to your project page from your documentation page.

Misc.
- your files have group write permissions, chmod 644 on all files

views_gsa_feed.info
- remove line 14-15 remove core= "7.x":

views_gsa_feed.install
- consider merging line 25 into single if: if (is_numeric($max_allowed_packet) && $max_allowed_packet < ( )) {
- extract 16 * 1024 * 1024 into a define statements for reputability and configurability

  define('VIEWS_GSA_FEED_MB', 1024 * 1024);
  define('VIEWS_GSA_FEED_MB_MIN_PACKET_MB', 16)
  ...
   .. max_allowed_packet < (VIEWS_GSA_FEED_MB * VIEWS_GSA_FEED_MB_MIN_PACKET_MB)  ..
  ...

- That way you can easily change up or down the minimum packet size without thinking about it

views_gsa_feed.module
- remove white space on line 12 - just a style thing

Testing
- Install instructions says "Save & continue" Should be "Continue & Edit".
- Step 1 of configuring, what is a datasource be? What are it's legal values?
- In configure steps merge step 3 - 4.
- module performs without errors and functions as documented!

The code clean and well written, nice module!

adding in my first review

manual review:

1. "module_load_include('inc', 'views_gsa_feed', 'theme/views_gsa_feed.theme');": Also, no need to use module_load_include as you are including files from your own module which you already know where they are. Use something like require_once 'mymodule.inc';
2. views_gsa_feed_views_api(): Views version 2 does not exist on Drupal 7.
3. views_gsa_feed_query_alter(): why is that function needed? Please add a comment.
4. "htmlspecialchars($view->style_plugin->rendered_fields[$num][$id]);": use check_plain() instead.
5. "public function get_style_type() {return 'gsa_feed';}": function body should be on a line by itself.
6. I think you should add a dependency to views_data_export so that you don't have to copy so much code. Overriding a custom XML feed template should be fairly easy.

But that are not application blockers, so I guess this is RTBC. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

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

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

Thanks for your contribution, tmctighe!

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.