Attached patch against HEAD resolves the following issues:

  • according to http://drupal.org/node/140311, menu strings should no longer be wrapped in t().
  • capitalize SQL keywords.
  • use ANSI standard <> instead of != in SQL to support more databases.
  • use standard of 2 spaces for indenting.
  • unnecessary newly declared variable for use on the following line only.
  • use db_query_range() for limit queries for wider database support.
  • trailing whitespace on lines.
CommentFileSizeAuthor
#9 v4.patch4.62 KBmokko
#6 diffagainstdev2.patch3.87 KBmokko
#5 mail_logger-v3.patch4.93 KBmokko
#2 mail_logger-rerolled.patch4.96 KBmokko
mail_logger-D6upgrade.patch4.66 KBdeekayen

Comments

litwol’s picture

litwol commented
Status: Needs review » Needs work

Please reroll and i'll commit asap. thx.

patching file mail_logger.install
patching file mail_logger.module
Hunk #1 FAILED at 1.
Hunk #2 succeeded at 15 (offset -9 lines).
Hunk #3 succeeded at 40 (offset -9 lines).
Hunk #4 succeeded at 66 (offset -9 lines).
Hunk #5 succeeded at 86 (offset -9 lines).
Hunk #6 succeeded at 133 (offset -9 lines).
Hunk #7 succeeded at 160 (offset -9 lines).
1 out of 7 hunks FAILED -- saving rejects to file mail_logger.module.rej

mokko’s picture

mokko commented
StatusFileSize
new mail_logger-rerolled.patch4.96 KB

Hm. When I downloaded dev and ran the patch against it I had the same output. Then I tried via cvs ceckout, the patch applied nicely. This is the first time I use cvs, so maybe I didn't get dev version or so. Anyways, here is a rerolled patch.

mokko’s picture

mokko commented

Now, i downloaded fresh dev version on devel server and try the patch against dev. Same result as before (like in your last post). So, let me look for error.

litwol’s picture

litwol commented
Version: 6.x-1.0 » 6.x-1.x-dev

make sure you use the appropriate branch. when checking out from cvs specify branch revision by using '-r DRUPAL-6--1'

mokko’s picture

mokko commented
Version: 6.x-1.x-dev » 6.x-1.0
StatusFileSize
new mail_logger-v3.patch4.93 KB

Another attempt. This time I made sure to checkout HEAD. Old patch still applies perfectly. Rerolled again. Looks the same. Very suspicious. Any ideas?

mokko’s picture

mokko commented
StatusFileSize
new diffagainstdev2.patch3.87 KB

This time I created the patch against dev. Let's see how this looks like.

mokko’s picture

mokko commented
Status: Needs work » Needs review

This last patch works on my devel server on a fresh dev version,i.e. the patch applies smoothly. Coder shows me less warnings than before and admin/reports/mail-logger pretty much looks like before. So far so good. What do you say?

litwol’s picture

litwol commented
Version: 6.x-1.0 » 6.x-1.x-dev

DId you use this cvs checkout command to fetch source ? it is important that you have used the correct branch name.

cvs -z6 checkout -d mail_logger-DRUPAL-6--1 -r DRUPAL-6--1 contributions/modules/mail_logger/

mokko’s picture

mokko commented
StatusFileSize
new v4.patch4.62 KB

Ok. This time I got you. I checked out with command you gave me. I applied the old patch, got the same partial success message that you got. I fixed the remaining one place by hand, and created a new patch. Here it is.

It should be functionally equivalent to the original patch, except that I changed the capitalization of another SQL command at the end of mail_logger.install.

Let's see what happens this time.

EDIT: Like before I can apply on my test server with fresh dev version. And functionality seems okay.

litwol’s picture

litwol commented
Status: Needs review » Fixed

Committed. thx.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.