Here's a patch with some code cleanup I needed/ran into while working on #1277484: --no-core option for archive-dump

CommentFileSizeAuthor
no_core_option_for_archive_dump-1277484-18-cleanup.patch5.02 KBhelmo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Status: Needs review » Fixed

Committed except for line below since it adds no useful information that the ssertion does not already provide. Those messages should be omitted when possible (this is a newish policy).

-    $this->execute('test -f ' . $untar_dest . '/MANIFEST.ini');
+    $this->assertFileExists($untar_dest . '/MANIFEST.ini', 'MANIFEST.ini should exist');
helmo’s picture

helmo CreditAttribution: helmo commented

Thanks, I'll remove the message parameter from my new patch in #1277484: --no-core option for archive-dump

Changing it to use assertFileExists instead of 'test' still seems like a good idea.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

I just changed two lines to use assertFileExists(). Thanks.

Status: Fixed » Closed (fixed)

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