ARCHIVE - 2009-04-25 & 26 Core patch review sprint

Last updated March 26, 2012. Created by webchick on April 25, 2009.
Edited by jhodgdon, dww, pwolanin, eojthebrave. Log in to edit this page.

This is just a "wiki" page to keep track of various happenings at the patch review sprint.

Patches reviewed

1. #392706-22: Move database assumptions from field.test to field_sql_module.test (webchick)
2. #369007-34: Don't rename user pictures (webchick)
3. #335756: SimpleTest: $this->originalFileDirectory should be set before installation to ensure non-standard directories are picked up (DamZ)
4. #228283-11: Patch for ANSI compatible: forum.module (Berdir)
5. #301362-79: Default to InnoDB in MySQL (webchick, chx, DamZ, stewsnooze, Moshe, Dries)
6. #302268-35: TableSorts and PagerDefault queries broken (Berdir)
7. #444402: Remove cruft from JavaScript code (caktux, webchick)
8. #444344: jQuery .once() method for adding behaviors once (caktux)
9. #427648: Comment clarification in drupal_get_title (caktux)
10. #434942: Vertical-tab-ify admin/user/settings (webchick)
11. #335756: SimpleTest: $this->originalFileDirectory should be set before installation to ensure non-standard directories are picked up (Dries)
12. #440826: System.module whitespace & coding style cleanup (Dries)
13. #345591: drupal_http_request should return the original status message and protocol (Dries)
14. #440768: Transfer the {node_counter} table from node.module (Dries)
15. #251792: Implement a locking framework for long operations (Moshe)
16. #394146: DBTNG filter.module (dropcube, Dries)
17. #422362-16: convert form.inc to use new static caching API (pwolanin)
18. #422368-5: convert menu.inc to use new static caching API (pwolanin)
19. #444778: Install.php overrides extra database connections (Moshe, webchick)
20. #427648: Comment clarification in drupal_get_title (webchick)
21. #383748: confirm_form() broken when destination URL contains query string (webchick)
22. #444352: Kill the killswitch (Dries, jjeff, markus_petrux, dmitrig01, quicksketch)
23. #303965: Enhanced data import - node_save() and user_save() (webchick)
24. #431976: Add wrapper function for current url (i.e., $_GET['q']) (chx, fransk)
25. #352880: Replace strstr with strpos (chx)
26. #428744-28: Make the main page content a real block (Berdir)
27. #363580: OpenID login fails when in maintenance mode (frega, webchick)
28. #432864: Improve comments for function base_path() (fransk
29. #444920: Book breadcrumbs are broken (chx, webchick)
30. #235673: Changes to block caching mode not caught (Berdir)
31. #251255: Taxonomy term synonyms and relations clear after re-ordering using drag-and-drop feature (Berdir)
32. #696-39: Add filter to logs and referrer lists (Berdir)
33. #339210: Support HTTP authentication (boombatower, webchick)
34. #353847: Tests for node_access (j.somers, dereine, webchick)
35. #356445: "Add new comment" link disappears after the first comment has been created. (Arancaytar)
36. #246398: Add path aliases to search index (Arancaytar)
37. #335185: Add js-show CSS class for hiding elements when JavaScript is disabled (Arancaytar)
38. #39432: Disallow anonymous voting when page caching is turned on (Arancaytar)
39. #365554: Fix colspan in tableselect (Arancaytar)
40. #362852: Clean up statistics admin settings (Arancaytar, webchick)
41. #369409: Move block theme definition from system module to block module (eojthebrave, webchick)
42. #19837: make (not verified) optional (xmacinfo)
43. #270685: Problems with appearance of Title and Body fields in content type editor (xmacinfo, webchick)
44. #445094: broken d_w_r() call on non-MySQL db in field_test.module (yched, Moshe, webchick)
45. #431148: Make it easier to visually distinguish security updates on Updates report (dww, webchick)
46. #30270: Printer-friendly version with copyright section (site_footer) (dmitrig01, webchick)
47. #390774: node_view still mentioned in API docs (eojthebrave, webchick)
48. #251245: openid.module needs tests (DamZ, chx, webchick)
49. #133188: Line break converter can result in empty node display - PCRE limits (chx, webchick)
50. #166025: Allow themers to alter body classes (Arancaytar, webchick)
51. #391340: Job queue API (chx, berdir, boombatower, dww)
52. #183678: Select Category via URL in Contact Form (caktux)
53. #58224: Allow anonymous users access to a members personal contact form (pp)
54. #161354: Disabling a module which provides a text format leaves the text format behind (fransk)(pp)
55. #373606: file.inc documentation needs overhaul (Arancaytar)(fransk)
56. #137383: fully qualified url (Arancaytar)
57. #376424: Submit AHAH forms when Enter is pressed in field. (Arancaytar)
58. #367343: Different load arguments for multiple % wildcards in path. (Arancaytar)
59. #267484: Incorrect filter documentation (Arancaytar)
60. #371897: Author of node links to current logged in user (Arancaytar)
61. #431976: Add wrapper function for current url (i.e., $_GET['q']) (Arancaytar)
62. #349508: Require UTF8 database encoding (Arancaytar)
63. #335928: Thai vowels are excluded in search index (Arancaytar)
64. #282852: "Non-public" background overlaps messages area (Arancaytar)
65. #394306: Syntax cleanup in simpletests (Arancaytar)
66. #392582: Module anchor links incorrect in admin/user/permissions (Arancaytar)
67. #276569: TestingParty08: Book block and links (Arancaytar)
68. #394676: PHP notices when new user logs in via openID (Arancaytar)
69. #296115: TestingParty08: menu router item titles need a test (Arancaytar)
70. #324432: Test for taxonomy filter on admin/content/node (Arancaytar)
71. #395916: Improve module_load_include to allow files that always need included (Arancaytar)
72. #383220: Cron trigger needs an action (Arancaytar)
73. #323926: Settings for non-existent theme (Arancaytar)
74. #346095: Test #ajax (DamZ)
75. #370835: Improve password checker (eojthebrave)
76. #193482: Styling status messages in system.css (eojthebrave)
77. #266153: Help text for block configuration on user/.../edit (eojthebrave)
78. #277018: Warning: Invalid argument supplied for foreach() in menu_unserialize() if unserialize returns non-array value (pwolanin)

Patches committed

1. #336475: simpletests for sticky tableheaders
2. #335756: SimpleTest: $this->originalFileDirectory should be set before installation to ensure non-standard directories are picked up
3. #440826: System.module whitespace & coding style cleanup
4. #345591: drupal_http_request should return the original status message and protocol
5. #440768: Transfer the {node_counter} table from node.module
6. #301362: Default to InnoDB in MySQL (wahoo!)
7. #394484: DBTNG: Node module
8. #444778: Install.php overrides extra database connections
9. #303965: Enhanced data import - node_save() and user_save()
10. #394146: DBTNG filter.module
11. #339210: Support HTTP authentication
12. #444920: Book breadcrumbs are broken
13. #444352: Kill the killswitch
14. #369409: Move block theme definition from system module to block module
15. #270685: Problems with appearance of Title and Body fields in content type editor
16. #422368-5: convert menu.inc to use new static caching API
17. #301362-79: Default to InnoDB in MySQL
18. #352880: Replace strstr with strpos
19. #19837: make (not verified) optional
20. #444966: Codestyle: Return of user_build_filter_query
21. #445070: Doxygen comment for system_settings_form() does not list all parameters.
22. #267484: Incorrect filter documentation
23. #193482: Styling status messages in system.css
24. #445414: format_interval doesn't format monthly intervals correctly

Patches made during the sprint (people just can't read sprint titles)

1. #444744: Include .module file during hook_install (mikejoconnor)
2. #444920: Book breadcrumbs are broken (DamZ)
3. #444876: Do we need node_invoke_node() ? (DamZ)
4. #220592: Core cache API breaks update.module: fetches data way too often, kills site performance, etc (dww)
5. #445748: Add hook_update_projects_alter() (Dave Reid, dww)
6. #441148-14: Improve node API's developer experience (DamZ)

Stuff folks had issues with

• dropcube had the issue at #357970: Undefined function token_get_all. Drupal 7 requires the php tokenizer extension. A note has been added to http://drupal.org/requirements. TODO: ask the docs team how best to version this page. TODO: Get this in a hook_requirements() so you can't go further (Patch: #445062: system_requirements should check for tokenizer extension).
• robomalo (and others) asked how to write tests: http://drupal.org/simpletest
• A few of the patches were "stuck" in requesting re-test mode (esp. around April 13 -- apparently an unlucky day). We got around this by asking participants to re-upload the patch to jog testing bot's memory.
• eojthebrave asked if a patch applies with offset, ie "Hunk #1 succeeded at 3317 (offset 26 lines).", is that acceptable or does it need to be re-rolled? TODO: find the spot in the docs that talks about this and link it to the patch review sprint page.
• timmillwood asked about a patch that testing bot gave approval to that caused bucket loads of errors on his Drupal installation. This patch made a change to hook_theme(), and so the cache needed to be cleared.

Money quotes

Patch review can really be quite fun and relaxing. /You/ get to tell that poor coder all the things that are wrong with the patch, instead of the other way around. :P

-- Arancaytar

Thanks for these two days; I learned a lot. I thought it was a great team, but now I know it.

-- pp