Files: 
CommentFileSizeAuthor
#61 1797252-61-t-file.patch53.56 KBdcam
PASSED: [[SimpleTest]]: [MySQL] 40,216 pass(es).
[ View ]
#57 1797252-55-t-file.patch106.43 KBpwieck
PASSED: [[SimpleTest]]: [MySQL] 57,975 pass(es).
[ View ]
#55 1797252-55-t-file .patch106.43 KBpwieck
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1797252-55-t-file .patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#53 1797252-53-t-file .patch106.43 KBpwieck
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1797252-53-t-file .patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#47 1797252-47-t-file.patch106.41 KBpwieck
FAILED: [[SimpleTest]]: [MySQL] 57,961 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#45 1797252-42-t-file.patch106.41 KBpwieck
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/file/lib/Drupal/file/Tests/SaveUploadTest.php.
[ View ]
#42 1797252-42-t-file.patch106.41 KBpwieck
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/file/lib/Drupal/file/Tests/SaveTest.php.
[ View ]
#38 1797252-38-t-file.patch104.9 KBdcam
PASSED: [[SimpleTest]]: [MySQL] 55,264 pass(es).
[ View ]
#32 1797252-32-t-file.patch105.73 KBdcam
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/file/lib/Drupal/file/Tests/FileFieldValidateTest.php.
[ View ]
#26 1797252-t-file-26.patch105.73 KBandypost
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1797252-t-file-26.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#22 1797252-t-file-22.patch104.61 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 55,357 pass(es).
[ View ]
#18 1797252-t-file-18.patch104.62 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 54,936 pass(es).
[ View ]
#13 interdiff.txt973 bytesandypost
#13 1797252-t-file-13.patch104.02 KBandypost
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1797252-t-file-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#11 t-file-1799752-11.patch104.07 KBdcam
PASSED: [[SimpleTest]]: [MySQL] 52,257 pass(es).
[ View ]
#7 file-1799752-7.patch103.84 KBdcam
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch file-1799752-7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#6 1799752-6-t-file.patch107.27 KBLars Toomre
PASSED: [[SimpleTest]]: [MySQL] 41,898 pass(es).
[ View ]
#1 file-1797252-1.patch70.1 KBxjm
PASSED: [[SimpleTest]]: [MySQL] 41,696 pass(es).
[ View ]

Comments

Assigned:xjm» Unassigned
Status:Active» Needs review
Issue tags:+needs backport to D7
StatusFileSize
new70.1 KB
PASSED: [[SimpleTest]]: [MySQL] 41,696 pass(es).
[ View ]

Larger one. No format_string().

git diff --color-words to review.

I did not apply this patch yet, but it appears several straight removals were missed. Shouldn't these be included?

+++ b/core/modules/file/lib/Drupal/file/Tests/SaveDataTest.phpundefined
@@ -26,12 +26,12 @@ class SaveDataTest extends FileManagedTestBase {
     $this->assertEqual(file_default_scheme(), file_uri_scheme($result->uri), t("File was placed in Drupal's files directory."));
     $this->assertEqual($result->filename, drupal_basename($result->uri), t("Filename was set to the file's basename."));
-    $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of the file are correct.'));
-    $this->assertEqual($result->filemime, 'application/octet-stream', t('A MIME type was set.'));
+    $this->assertEqual($contents, file_get_contents($result->uri), 'Contents of the file are correct.');
+    $this->assertEqual($result->filemime, 'application/octet-stream', 'A MIME type was set.');
     $this->assertEqual($result->status, FILE_STATUS_PERMANENT, t("The file's status was set to permanent."));

I am not sure what was going on here... Looks like several straight t() removals were missed her. Am I wrong? Also down below in this test too.

Component:dblog.module» file.module

These weren't matched by the script because they contained apostrophes, and I didn't go through every file by hand for this issue because I'd already had to proof 70 K of changes. Feel free to update the patch to add them.

Also, correcting the component.

Ah, thanks... All of the files on this topic I generated by hand. I did not realize you were using a script. Thanks.

See #500866-209: [META] remove t() from assert message. It's very conservative in order to avoid false positives, other than the false positive with the $group parameter, which is why I assumed the other issues had those! :) So you still need to check its changes by hand, and for a smaller set of tests I still have been going through every use of t() to see which can be removed.

StatusFileSize
new107.27 KB
PASSED: [[SimpleTest]]: [MySQL] 41,898 pass(es).
[ View ]

Thanks for your explanation in #5 @xjm.

I took the scripted patch results from #1, applied it locally, and then went through and did a hand review of all resulting t() occurances that remained. Those that needed to be converted, including those that needed format_string() conversion, have now been done so. The results are in this now complete patch, at least for me locally according to grep.

Note: I did not check for all assert messages (which might have included one or more string concatenations, which should be converted to format_string()). I figured that this patch was going to be big enough.

I have not yet tested this so let's see what the test bots think.

StatusFileSize
new103.84 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch file-1799752-7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

#6 needed to be rerolled due to changes in DeleteTest.php and UsageTest.php

I checked all the asserts after rerolling it. I didn't find any more t()'s in the messages. I can't RTBC it though due to the reroll.

Thanks for the re-roll @dcam. Unfortunately, I do not think I can RTBC this one either as I rolled the expanded patch in #6.

Issue tags:-needs backport to D7

#7: file-1799752-7.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+needs backport to D7

The last submitted patch, file-1799752-7.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new104.07 KB
PASSED: [[SimpleTest]]: [MySQL] 52,257 pass(es).
[ View ]

Rerolled #7.

Issue tags:+Novice

Tagging as Novice.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new104.02 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1797252-t-file-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new973 bytes

Re-roll for current HEAD
Found small issue once reviewed

Assigned:Unassigned» jhodgdon

This patch touches some test files that an "avoid commit conflicts" issue also touches, so I'm going to wait on committing it until
#1888424: Make Drupal's URL generation logic available to HttpKernel, and minimize code repetition/divergence
is taken care of. Unfortunately, it might need to be rerolled at that point.

#13: 1797252-t-file-13.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work
Issue tags:+Novice, +needs backport to D7

The last submitted patch, 1797252-t-file-13.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new104.62 KB
PASSED: [[SimpleTest]]: [MySQL] 54,936 pass(es).
[ View ]

Re-roll for current head

RE #19 - that other issue is tagged "avoid commit conflicts", so this one will have to wait. Even if it is temporarily at "needs work", I would not commit this issue until it is done, as it would likely impede progress.

Assigned:jhodgdon» Unassigned

Actually I'm unassigning this until it gets reviewed.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new104.61 KB
PASSED: [[SimpleTest]]: [MySQL] 55,357 pass(es).
[ View ]

re-roll again, there's nothing to review just simple merge
Also it could hev conflicts with #1875992: Add EntityFormDisplay objects for entity forms

Assigned:Unassigned» jhodgdon

Thanks! Assigning to me to do final review/commit if/when there are no conflicts with "avoid commit conflicts" issues.

There's yet another "avoid commit conflicts" patch that touches some of the same files as this one and may conflict:
#731724: Convert comment settings into a field to make them work with CMI and non-node entities (the latest patch is on page 2 of the comments: http://drupal.org/node/731724?page=1 down at the bottom; the "latest comment" and "latest patch" links do not work on pages with more than 300 comments, which is a known issue)

So this will need to wait for commit until that other issue is resolved.

@jhodgdon Patch still in progress see #1907960-177: Helper issue for "Comment field" and as one of authors I just dont want to add another comment to #731724: Convert comment settings into a field to make them work with CMI and non-node entities just to remove tag

StatusFileSize
new105.73 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1797252-t-file-26.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

re-roll

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 1797252-t-file-26.patch, failed testing.

Assigned:jhodgdon» Unassigned

Status:Needs work» Needs review
Issue tags:-Novice, -needs backport to D7

#26: 1797252-t-file-26.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Novice, +needs backport to D7

The last submitted patch, 1797252-t-file-26.patch, failed testing.

Issue tags:+Needs reroll

Tagging for reroll.

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new105.73 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/file/lib/Drupal/file/Tests/FileFieldValidateTest.php.
[ View ]

Rerolled #26.

Status:Needs review» Needs work
Issue tags:-Novice, -needs backport to D7

The last submitted patch, 1797252-32-t-file.patch, failed testing.

Status:Needs work» Needs review

#32: 1797252-32-t-file.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Novice, +needs backport to D7

The last submitted patch, 1797252-32-t-file.patch, failed testing.

Thanks @dcam!

FileFieldValidateTest.php and FileFieldWidgetTest.php have unmerged changes - I think #26 was not a complete merge.

e.g.:

+++ b/core/modules/file/lib/Drupal/file/Tests/FileFieldWidgetTest.phpundefined
@@ -216,22 +224,27 @@ function testPrivateFileSetting() {
+<<<<<<< HEAD
+    $node_file = file_load($node->{$field_name}[LANGUAGE_NOT_SPECIFIED][0]['fid']);
+    $this->assertFileExists($node_file, 'New file saved to disk on node creation.');
+=======
     $node_file = file_load($node->{$field_name}[Language::LANGCODE_NOT_SPECIFIED][0]['fid']);
     $this->assertFileExists($node_file, t('New file saved to disk on node creation.'));
+>>>>>>> 8.x

Oooh. Yeah. I knew I should have just checked the file. I'll get right on it.

Status:Needs work» Needs review
StatusFileSize
new104.9 KB
PASSED: [[SimpleTest]]: [MySQL] 55,264 pass(es).
[ View ]

Rerolled #22.

bump! We only have a couple of these left. Can someone review this patch?

Status:Needs review» Needs work

Patch needs re-roll but everythinf is fine

+++ b/core/modules/file/lib/Drupal/file/Tests/SaveUploadTest.phpundefined
@@ -131,10 +131,10 @@ function testHandleExtension() {
+    $this->assertRaw(t('Epic upload FAIL!'), 'Found the failure message.');
@@ -151,9 +151,9 @@ function testHandleExtension() {
+    $this->assertRaw(t('You WIN!'), 'Found the success message.');

funny!

Assigned:Unassigned» pwieck

Working on reroll.

Assigned:pwieck» Unassigned
Status:Needs work» Needs review
StatusFileSize
new106.41 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/file/lib/Drupal/file/Tests/SaveTest.php.
[ View ]

Reroll to current head. No diff file because it was almost a complete redo since head files had changed so much. The diff was as near as long as patch.

Status:Needs review» Needs work

The last submitted patch, 1797252-42-t-file.patch, failed testing.

Assigned:Unassigned» pwieck

My mistake. Fixing

Assigned:pwieck» Unassigned
Status:Needs work» Needs review
StatusFileSize
new106.41 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/file/lib/Drupal/file/Tests/SaveUploadTest.php.
[ View ]

Fixed
'Timestamp didn't go backwards.' --> "Timestamp didn't go backwards."

Status:Needs review» Needs work

The last submitted patch, 1797252-42-t-file.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new106.41 KB
FAILED: [[SimpleTest]]: [MySQL] 57,961 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

Holly cow! did it again

'The image file we're going to upload exists.'); --> "The image file we're going to upload exists.");

Status:Needs review» Needs work
Issue tags:-Novice, -needs backport to D7

The last submitted patch, 1797252-47-t-file.patch, failed testing.

Status:Needs work» Needs review

#47: 1797252-47-t-file.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 1797252-47-t-file.patch, failed testing.

Status:Needs work» Needs review

#47: 1797252-47-t-file.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Novice, +needs backport to D7

The last submitted patch, 1797252-47-t-file.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new106.43 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1797252-53-t-file .patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

missed a format_string. Sure taking the long road on this one. sigh :-(

Status:Needs review» Needs work

The last submitted patch, 1797252-53-t-file .patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new106.43 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1797252-55-t-file .patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Reroll to current head. No diff file because it was almost a complete redo since head files had changed so much. The diff was as near as long as patch.

Status:Needs review» Needs work
Issue tags:-Novice

The last submitted patch, 1797252-55-t-file .patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new106.43 KB
PASSED: [[SimpleTest]]: [MySQL] 57,975 pass(es).
[ View ]

I'm sooo tied I can't seem to get this one right. Keep putting space in file name.

Reroll to current head. No diff file because it was almost a complete redo since head files had changed so much. The diff was as near as long as patch.

Status:Needs review» Reviewed & tested by the community

@pwieck thanx a lot for serious re-roll, let's get this in

Status:Reviewed & tested by the community» Fixed

Committed 795e1f8 and pushed to 8.x. Thanks!

Version:8.x-dev» 7.x-dev
Status:Fixed» Patch (to be ported)

Status:Patch (to be ported)» Needs review
StatusFileSize
new53.56 KB
PASSED: [[SimpleTest]]: [MySQL] 40,216 pass(es).
[ View ]

Backported #57 to D7.

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

I've checked it, applies cleanly, passes tests and all looks good. RTBC.

Version:8.x-dev» 7.x-dev

Sorry, didn't mean to change version number. Back to 7.x still RTBC.

Status:Reviewed & tested by the community» Fixed

Thanks all! Committed to 7.x.

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