The attached patch cleans up the module and updates it to work with current HEAD (Drupal 5).

CommentFileSizeAuthor
#8 cm_1.patch19.61 KBkbahey
#7 cm_0.patch19.52 KBkbahey
#3 cm.patch19.83 KBkbahey
drupal-5_0.patch19.76 KBkkaefer

Comments

kbahey’s picture

Patch needs to change date_update_1() to commentmail_update_1().

Otherwise, look good.

quicksketch’s picture

dprint_r($comment_obj); also needs to be removed.

kbahey’s picture

Status: Needs work » Needs review
StatusFileSize
new19.83 KB

Here is a patch that removes the above two comments.

kvarnelis@drupal.org’s picture

didn't work.

upon visiting the settings page, i got ..

warning: call_user_func_array() [function.call-user-func-array]: First argumented is expected to be a valid callback, 'commentmail_admin_settings' was given in /Library/WebServer/Documents/drupal/includes/form.inc on line 218.

and
in trying to post i got


Fatal error: Call to undefined function: user_mail() in /Library/WebServer/Documents/drupal/modules/commentmail/commentmail.module on line 55
kvarnelis@drupal.org’s picture

actually it may be that the second patch is malformed... trying to decode it by hand...

kvarnelis@drupal.org’s picture

i wish i could edit the previous posts, but since i can't...

i have commentmail working, but don't use the second patch. it does not contain the patch for the .install file, which is what gave me the errors.
it also doesn't touch the dprint () which leads to a quick death.

manually changing the two comments did work.

kbahey’s picture

StatusFileSize
new19.52 KB

Here is another patch. It removes the dprint, corrects the name of the function in the .install file, has a .info file, and should work well.

kbahey’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new19.61 KB

Here is a better patch:

- Version number added to the .info file.
- Description added to the settings.

Marking as RTBC.

kbahey’s picture

I can confirm that the latest patch in #8 works well on a live site.

kkaefer’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

kbahey’s picture

Thanks Konstantin.

Do you plan to branch it and release it for 5.x soon?

kkaefer’s picture

I decided that as long as this module is adapted to a post-5 version, I keep developing it in HEAD. Once it's ported to 6.x(-dev), I'll branch it. Please test the HEAD version. If no errors occur, I'll release a 5.x-1.0 in the next couple of days.

kbahey’s picture

OK, I switched the 3 sites to the official HEAD from the repository.

One more thing: add this line in the .info file:

version = "$Name: $"

kkaefer’s picture

AFAIK, the release system automatically adds this line when a user downloads a tarball from the website. Power users using CVS checkouts can figure out the version from the Id string.

kbahey’s picture

It says here: http://drupal.org/node/101009

version (Optional)
The version string will ordinarily be added by drupal.org when a release is created and a tarball packaged. However, for users getting their modules directly from CVS, they will not have a version string. In order to accomodate them, you may use the following string:

version = "$Name$"

You should not put anything else here. Please note that the quotes are required; some PHP versions are more sensitive to the punctuation than others. It is often suggested to use $Revision$ here. Do Not Do This. It will give the revision of the .info module which is utterly useless.

Also keep in mind that, as a developer, if you use $Name$ you will see $Name$ if your checkout is from HEAD or you only just branched and have not done a new checkout. cvs update from the same branch will not consistently update $Name$, but sometimes it will appear to. If you see $Name$ and you think you should see a tag, do a cvs checkout.

To reiterate: Use of this field and $Name$ is only for the convenience of the author and users checking the module out of CVS.

So it is better to include it.

Anonymous’s picture

Status: Fixed » Closed (fixed)