Closed (fixed)
Project:
Drupal core
Version:
7.x-dev
Component:
simpletest.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
7 Mar 2009 at 20:16 UTC
Updated:
3 Jan 2014 at 00:07 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
damien tournoud commentedHere.
One call in drupal_web_test_case.php can't be converted right now, because we don't support data subqueries on INSERT right now:
I suggest we go ahead and deal with the rest first.
Comment #2
damien tournoud commentedNow with every inline query converted.
I also removed a test:
... for two reasons:
(1) We don't support any form of quoting in db_query(), so we shouldn't promote bad examples,
(2) It doesn't matter anyway, because PDO is taking care of quoting
Comment #3
dries commentedI reviewed this patch and it looks good. No comments. Just waiting for the test results from the test bot before committing this patch. Thanks Damien.
Comment #5
chx commentedRelated work #356074: Provide a sequences API
Comment #6
damien tournoud commentedRerolled.
Comment #7
berdirAccording to Crell, arrays with only a single value should be on the same line: #394484-3: DBTNG: Node module
According to the documentation, this can be written on a single line, because it's only one method and the array just contains the values: http://drupal.org/node/310079
But this should be array with keys, does this actually work ?
Just wondering if it is really necessary to pass in static args as parameters, I've never done that.
You don't touch that yet, but that should be multi-line.
You converted it to single quotes, but it still contains a %d param.
Most of the above things occur more than once in the patch, I've just picked an example. Also, you convert all queries to single quotes. It's not a bad thing, it just makes the patch huge :)
Comment #8
berdirUh oh, that one got even bigger :)
Changed my own points in #7 and also fixed a few DBTNG Coding Style issues, mostly with chaining and arrays.
Comment #9
dries commentedCommitted to CVS HEAD. Thanks!