Timefield

Can you explain what this does that can't be done with date module, and why a new module is necessary rather than making changes to date?

Marking as critical as per http://drupal.org/node/894256
Sorry for the long wait.

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:

• Lines in README.txt should not exceed 80 characters, see the guidelines for in-project documentation.
• ./modules/timefield_taxonomy.module: The description on the line after the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions
  

  43- */
  

• ./timefield.module: The description on the line after the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions
  

  420- */
  743- * the php date format
  745- * the integer value to be converted
  763- * time format that should be parsable via date_parse().
  781- * time format that should be parsable via date_parse().
  855- * The currentlt defined display settings, in PHP date() format.
  

• Bad line endings were found, always use unix style terminators. See http://drupal.org/coding-standards#indenting
  

  ./js/jquery.ui.timepicker.min.js:             UTF-8 Unicode text, with very long lines, with no line terminators
  
  

• All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
  

  ./modules/timefield_taxonomy.info ./css/jquery-ui-timepicker.css ./README.txt ./timefield.install ./js/jquery.ui.timepicker.min.js ./js/jquery.ui.timepicker.js ./theme/timefield-weekly-minical-box.tpl.php ./theme/timefield.tpl.php ./timefield.info ./views/timefield_plugin_style_minical.inc
  

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

Source: http://ventral.org/pareview - PAReview.sh online service

I just spotted a potential duplicate at http://drupal.org/project/cck_time

(Admittedly this proposal has the better module name now CCK is obsolete on D7...)

Review of the 7.x-1.x branch:

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

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.

manual review:

• jquery.ui.timepicker.js appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms. The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.
• _timefield_time_part_format(): not all text in there is run through t().
• Your views integration contains many debug statements and commented out code, please remove that. Also, drupalcs does not really like the Views coding style, you can ignore overridden method names that you cannot change.

Otherwise I think this is nearly ready.

There's still a few imperfections: http://ventral.org/pareview/httpgitdrupalorgsandboxwesnick1168872git

And something I consider a bug:

• These are my field instance settings:
  
• Which adds this to the input form:
  
  (which is acceptable, but incorrect IMO)
• And the formatter result is also a 12 hour clock including "pm". (I've seen the formatter settings in the Manage Display tab: kudo's.) What does the "Show or Hide AM/PM" option do??

1. A few notices when I edit the node or time field instance, but they're gone after saving the field instance settings. (You added the closeButton etc options.)

2. But the problem persists: when I enter "14:00" in the field instance's Default Value textfield, I get "2:00pm" when I reload that page, and also when I edit or add a node with that field. "Show AM/PM Label" is unchecked (verified after saving).

3. The first sentence in your README is a little unclear: "Timefield is a Field API field that stores a time value, with an optional to time." Quotes around "to time" or renaming it to "end time" (like some other places) would help.

It's an obvious addition though. Three date fields in the date module, but no decent time fields. Good options too.

I've only now tried the included Timefield Taxonomy module. There's definitely not enough information, because I don't get it =)

• I've created a vocabulary
• I've added a time field to that vocabulary
• I've created a term reference field in a node type linked to that vocabulary

And now what?

Can you make a list of steps the user has to follow to get the taxonomy part working? First step is enabling the module. Last step is adding or editing content.

Just one more thing:

Timefield module provides a field type 'timefield' with a default JQuery widget.

Where do I get the jQuery plugin? A message after install or a constant warning or something like that might be nice to let the user know he's not quite done yet.

And then you're done IMO. Good module.

Excellent.

You have committed a backup file README.txt~. You can set up you GIT so that it ignores all ~ files: http://help.github.com/ignore-files/

manual review:
"_timefield_display_format_form('column_format', "Column Time Settings", $settings);": all user facing text should run through t() for translation. Also elsewhere.

But that is just a minor issue, so ...

Thanks for your contribution, wesnick! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

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.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Congratulations!

(I've just hit your issue queue with a ton of minor bugs...! :D)