minor upgrade fixes [#342883]

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.

Comment	File	Size	Author
#9	v4.patch	4.62 KB	mokko
#6	diffagainstdev2.patch	3.87 KB	mokko
#5	mail_logger-v3.patch	4.93 KB	mokko
#2	mail_logger-rerolled.patch	4.96 KB	mokko
	mail_logger-D6upgrade.patch	4.66 KB	deekayen

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

litwol CreditAttribution: litwol commented 5 February 2010 at 20:41

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

Comment #2

mokko CreditAttribution: mokko commented 6 February 2010 at 14:08

File	Size
mail_logger-rerolled.patch	4.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.

Comment #3

mokko CreditAttribution: mokko commented 6 February 2010 at 14:30

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.

Comment #4

litwol CreditAttribution: litwol commented 6 February 2010 at 14:22

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'

Comment #5

mokko CreditAttribution: mokko commented 6 February 2010 at 14:26

Version:

6.x-1.x-dev

» 6.x-1.0

File	Size
mail_logger-v3.patch	4.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?

Comment #6

mokko CreditAttribution: mokko commented 6 February 2010 at 14:39

File	Size
diffagainstdev2.patch	3.87 KB

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

Comment #7

mokko CreditAttribution: mokko commented 6 February 2010 at 14:48

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?

Comment #8

litwol CreditAttribution: litwol commented 6 February 2010 at 15:32

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/

Comment #9

mokko CreditAttribution: mokko commented 6 February 2010 at 22:25

File	Size
v4.patch	4.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.