pianomansam [pianomansam]

1.       // Return a line-break version of the module README
         return filter_filter('process', 2, NULL, file_get_contents( dirname(__FILE__) . "/README.txt") );
   

   There is a Drupal function to get the actually directory where a module is installed.

2. function teaser_input_format_install(){
   	db_query("ALTER TABLE {filter_formats} ADD teasers VARCHAR(256)");
   	drupal_set_message(st("Teaser Input Format has been installed."));
   }
   
   
   function teaser_input_format_uninstall() {
   	db_query("ALTER TABLE {filter_formats} DROP teasers");
   }
   

   There are Drupal functions thought to alter an existing database table.

3.       "#title" => t("Teaser Format"),
         // "#description" => t("");
   

   Strings used in user interface should be in sentence case.

4. ; Information added by drupal.org packaging script on 2009-05-20
   version = "6.x-0.1"
   core = "6.x"
   project = "teaser_input_format"
   datestamp = "1242789632"
   

   Those lines are already added by the packaging script, and should be removed.

There have not been replies from the OP in the past 7 days. I am marking this report as won't fix.

@pianomansam, thanks for the application. The following points are just a start and do not encompass all of the changes that may be necessary for your approval. Also, a specific point may just be an example and may apply in other places.

1. Overall, adhere to Drupal Coding Standards. This includes more than just PHP files. This includes docblocks for all files and functions. Also, look at using Coder to help automate SOME of this.
2. Remove old code in .install file.
3. Fill out the README.txt
4. I would not suggest reading in the README.txt for the file. It will be too much. The hook_help() for admin/help should be a general description and tell the user about where help is, like the README.txt or in the input format pages.

5.   //Alter node edit form to rename and check/disable Teaser Include checkbox.
   

   There should be a space between the slashes and the comment. Also, keep comments no longer than 80 characters wide, taking into account indents.

6.     $teaser_format = db_fetch_object(db_query('SELECT format as id FROM {filter_formats} WHERE teasers LIKE "%%s%"', $type));

   All SQL keywords should be all caps, specifically "as" here.

7. <?
   if ($form["#id"] == "node-form") {
   ?>
   This is what $form_id is for.
8. Look at using http://api.drupal.org/api/function/hook_form_FORM_ID_alter/6 instead of hook_form_alter() as it is fired before hook_form_alter() and provides a slight performance increase as it is only called when needed.
9. Not really sure if this is stated anywhere, but it is a really bad idea to alter Drupal core tables. This could cause problems upgrading and it ruins the idea of maintaining a schema. You should, as with most all node modules, create a new table that you can join with the filter_format table. Also, this would make for a much cleaner implementation, instead of storing serialized data.

10.       if (!$teaser) {
            $body = $node->content["body"]["#value"];
            $node->content["body"]["#value"] = str_replace($node->teaser, "", $body);
          }
    

    Why would you do this? The teaser is part of the body of the content in Drupal.

11. It seems like your logic is that you are taking the teaser out of the body and applying the custom filter and putting back in the body. This is very destructive and not following the usual paradigm of Drupal which is filter the output. You should simply apply the custom filter on output when it is the teaser view. Maybe I am missing something.
12. Also, as with how filters are currently handled, you should make it so that each node has the ability to pick the filter for the teaser.

Oerall, pretty good, but I think you are going about this a little wrong. You should think about how Drupal would implement this if it was in core and how it can be the most flexible for other people.

--
Note: Please be patient with the CVS application process. It is all done by volunteers. Our goal is not to be arbitrarily slow or meticulous. Our goal is to get you CVS access and ensure that you are and will become a more responsible Drupal contributor. For a quick reference on what I look for in a code review, please see this article on what I look for.

There have not been replies in the past week. I am marking this application as won't fix.

- define 1st parameter should be string.

define(PROFILE_FIELD, 'profile_week');

- Your files (.info, .module) must put $Id$ in the header for CVS info.

- You should implement admin interface now, unless the user must put their custom fields by hard code to the define statement in your module file.

Hello pianomansam,

Tremendous apologies for the ridiculous delay here. Your module switch does make the review process more difficult :-)

I don't think it's documented anywhere, but it's an unwritten rule that modules hosted on drupal.org should not require any hacking (just like Drupal itself does not). In other words, if values should be changed, that (admittedly annoying to build) user interface is needed. (Personally, i'd rather we had a way to do that with config files and i hope we do some cool things with an API for settings in Drupal 8!)

As in zzolo's review, i think your first-submitted module is almost there.

Since the process is officially supposed to be evaluating applicants (currently, once you have access, projects don't again get a formal review), i am marking your CVS application as ready to be granted based on understanding of code, following of coding standards, and community awareness in trying to solve problems.

When you finally do get access— it will happen; and when we escape from CVS this won't even be an issue to get started— we know this process is bad and we're working to make it better — when you finally get access, try to only post projects you expect to be able to do a pretty complete job on finishing.

Congrats, you are in!

Please read the following resources to make sure you know how to use CVS and the specifics to the Drupal CVS infrastructure, as well as how to be a good module maintainer on Drupal.org. The Drupal community is very large and dynamic; we welcome you as a module maintainer and hope that you embrace and challenge the Drupal community and continue to contribute.

• CVS reference: http://drupal.org/handbook/cvs/quickstart
• Maintaining modules: http://drupal.org/node/7765