common.inc is nice and simple, with not much to convert.
OK, not entirely true. There's some logic in drupal_error_handler() that is dependent on the old database system for determining whether an error occurred "inside" the DB layer that I don't think applies anymore, but there are other patches already in the queue to overhaul that function anyway. I also didn't touch the horror that is drupal_write_record(), as I am not convinced that we even need to keep that function. We'll find out after refactoring the rest of Drupal to see if db_insert() and db_merge() can reasonably replace it (I think they can). If not, we can convert it then but for now it's not worth the time. That makes this a low-kitten-count patch. :-)
Comment | File | Size | Author |
---|---|---|---|
#5 | common_inc.patch | 3.58 KB | Crell |
common_inc.patch | 1.7 KB | Crell | |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedCode looks good. Need to test.
OT: Why does the return in flood_is_allowed do
return ($number < $threshold ? TRUE : FALSE);
instead ofreturn ($number < $threshold);
?Comment #2
Crell CreditAttribution: Crell commentedUm. I have no idea. The original code did, and I didn't want to harm any kittens in this patch. :-)
Comment #3
Dave ReidSeparate patch for the TRUE : FALSE in #319666: Remove unnecessary boolean ternary conditionals. I found a few more of them throughout core.
Comment #4
Senpai CreditAttribution: Senpai commentedFail. Visiting user/3/contact while logged in as uid3 results in a PDO exception.
Incidentally, there were 0 lines in {flood} when I started the test.
Comment #5
Crell CreditAttribution: Crell commentedYar. Fixed. I also removed the redundant ternary while I was at it.
Comment #6
Dries CreditAttribution: Dries commentedI committed this to CVS HEAD. Thanks Crell and Dave.
Dave, can you create an issue for the fact that the SimpleTests did not catch the first regression? Sounds like we're not testing the flood system.
Comment #7
Dries CreditAttribution: Dries commentedComment #9
Crell CreditAttribution: Crell commentedTagging.