See patch below.

Files: 
CommentFileSizeAuthor
#16 drupal-grammar-bootstramp.inc-d7-2038059-16.patch1.63 KBElijah Lynn
PASSED: [[SimpleTest]]: [MySQL] 40,347 pass(es).
[ View ]
#12 drupal-grammar-bootstrap-inc-2038059-10.patch2.71 KBElijah Lynn
PASSED: [[SimpleTest]]: [MySQL] 57,239 pass(es).
[ View ]
#10 drupal-grammar-bootstrap-inc-2038059-10.patch1.86 KBElijah Lynn
PASSED: [[SimpleTest]]: [MySQL] 56,575 pass(es).
[ View ]
#5 drupal-grammar-bootstrap-inc-2038059-5.patch972 bytesElijah Lynn
PASSED: [[SimpleTest]]: [MySQL] 57,045 pass(es).
[ View ]
#6 drupal-grammar-bootstramp.inc-2038059-6.patch1.63 KBElijah Lynn
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-grammar-bootstramp.inc-2038059-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 drupal-grammar-bootstramp.inc-2038059-1.patch1.63 KBElijah Lynn
PASSED: [[SimpleTest]]: [MySQL] 40,341 pass(es).
[ View ]

Comments

StatusFileSize
new1.63 KB
PASSED: [[SimpleTest]]: [MySQL] 40,341 pass(es).
[ View ]

Status:Active» Needs review

Status:Needs review» Reviewed & tested by the community

Patch applies cleanly.

Looks good to me.

Version:7.x-dev» 8.x-dev
Status:Reviewed & tested by the community» Needs work

Thanks! This all looks good except:

- *   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.

Status:Needs work» Needs review
StatusFileSize
new972 bytes
PASSED: [[SimpleTest]]: [MySQL] 57,045 pass(es).
[ View ]

Thanks 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.

Status:Needs work» Needs review
StatusFileSize
new1.63 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-grammar-bootstramp.inc-2038059-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Here is the updated patch for D7 (minus e.g.).

Status:Needs review» Needs work

The last submitted patch, drupal-grammar-bootstramp.inc-2038059-6.patch, failed testing.

For future reference, is there anything I can add to a patch to have it test against a certain version of Drupal?

Status:Needs review» Needs work

RE #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!

StatusFileSize
new1.86 KB
PASSED: [[SimpleTest]]: [MySQL] 56,575 pass(es).
[ View ]

Thanks for the tip on no testing trick.

Attached is a patch for D8 that also corrects CacheArray and CacheCollector.

Status:Needs work» Needs review

StatusFileSize
new2.71 KB
PASSED: [[SimpleTest]]: [MySQL] 57,239 pass(es).
[ View ]

That was a bad patch, use this instead.

Status:Needs review» Reviewed & tested by the community

Thanks! 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).

Thanks Jennifer!

And thanks for the explanation on waiting for the D7 patch, make perfect sense!

Cheers,

Elijah

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Thanks again! Committed to 8.x. Ready for backport to 7.x now.

StatusFileSize
new1.63 KB
PASSED: [[SimpleTest]]: [MySQL] 40,347 pass(es).
[ View ]

Thanks Jennifer,

Here is the same patch from #6.

Cheers!

Status:Patch (to be ported)» Needs review

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Fixed

Thanks again! Committed to 7.x.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.