Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
- * The type of the item (i.e. theme, theme_engine, module, profile).
+ * The type of the item (e.g. theme, theme_engine, module, profile).
People are always mistaking i.e. for e.g. (or vice versa) and they are nearly always (as in this case) also punctuated incorrectly (there needs to be a comma after either i.e. or e.g.). So please change it to read "(for example, theme, theme_engine, module, or profile). Actually though... come to think of it, i.e. is actually correct here, since this function is not supposed to work for anything except for those 4 choices. So let's actually just take the i.e. out entirely, because then it will be even clearer that these are the only 4 choices.
Also, by policy we need to make these fixes in Drupal 8.x first and then backport to 7.x. At least one of them is in 8.x boostrap.inc; the others may be in other files.
The first two in DrupalCacheArray::persist do not exist in the D8 bootstrap.inc but the last two in drupal_get_filename() do exist. Here is a patch for those last two in D8.
If you feel you must upload a patch for a different version ahead of time, there is a suffix to make it not test (it is described in the help text under the Attachment field on issue comments).
Regarding the patch in #5, CacheArray and CacheCollector have the same problems in Drupal 8 (they are just in different files, not in bootstrap.inc) that you identified in Drupal 7. So please patch those files. Thanks!
Please wait to make the D7 patch until that's done, because if you upload a D7 "do not test" patch now, someone will need to download your patch, change the name, and re-upload in order for it to be tested (and it needs to be tested before it can be committed, even though it's just API docs).
Comments
Comment #1
Elijah LynnComment #2
Elijah LynnComment #3
aaronott CreditAttribution: aaronott commentedPatch applies cleanly.
Looks good to me.
Comment #4
jhodgdonThanks! This all looks good except:
People are always mistaking i.e. for e.g. (or vice versa) and they are nearly always (as in this case) also punctuated incorrectly (there needs to be a comma after either i.e. or e.g.). So please change it to read "(for example, theme, theme_engine, module, or profile). Actually though... come to think of it, i.e. is actually correct here, since this function is not supposed to work for anything except for those 4 choices. So let's actually just take the i.e. out entirely, because then it will be even clearer that these are the only 4 choices.
Also, by policy we need to make these fixes in Drupal 8.x first and then backport to 7.x. At least one of them is in 8.x boostrap.inc; the others may be in other files.
Comment #5
Elijah LynnThanks Jennifer,
The first two in DrupalCacheArray::persist do not exist in the D8 bootstrap.inc but the last two in drupal_get_filename() do exist. Here is a patch for those last two in D8.
Comment #6
Elijah LynnHere is the updated patch for D7 (minus e.g.).
Comment #8
Elijah LynnFor future reference, is there anything I can add to a patch to have it test against a certain version of Drupal?
Comment #9
jhodgdonRE #8: no. Uploading patches to the wrong version is discouraged -- see https://drupal.org/node/1319154#multiple-versions
If you feel you must upload a patch for a different version ahead of time, there is a suffix to make it not test (it is described in the help text under the Attachment field on issue comments).
Regarding the patch in #5, CacheArray and CacheCollector have the same problems in Drupal 8 (they are just in different files, not in bootstrap.inc) that you identified in Drupal 7. So please patch those files. Thanks!
Comment #10
Elijah LynnThanks for the tip on no testing trick.
Attached is a patch for D8 that also corrects CacheArray and CacheCollector.
Comment #11
Elijah LynnComment #12
Elijah LynnThat was a bad patch, use this instead.
Comment #13
jhodgdonThanks! I'll get this patch committed shortly.
Please wait to make the D7 patch until that's done, because if you upload a D7 "do not test" patch now, someone will need to download your patch, change the name, and re-upload in order for it to be tested (and it needs to be tested before it can be committed, even though it's just API docs).
Comment #14
Elijah LynnThanks Jennifer!
And thanks for the explanation on waiting for the D7 patch, make perfect sense!
Cheers,
Elijah
Comment #15
jhodgdonThanks again! Committed to 8.x. Ready for backport to 7.x now.
Comment #16
Elijah LynnThanks Jennifer,
Here is the same patch from #6.
Cheers!
Comment #17
Elijah LynnComment #18
longwaveComment #19
jhodgdonThanks again! Committed to 7.x.