feat(storage): support delete source objects on compose#2224
feat(storage): support delete source objects on compose#2224nidhiii-27 wants to merge 3 commits into
Conversation
Updates compose file sample to support deleting source objects optionally. Fixes b/441557254 [Generated-by: AI]
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds an optional $deleteSourceObjects parameter to the compose_file function, enabling the deletion of source objects after composition. The test suite has been updated with a data provider to cover both cases. The review feedback suggests replacing the assertEquals calls on boolean values with more idiomatic assertTrue and assertFalse assertions in the tests.
| $this->assertEquals(!$deleteSourceObjects, $bucket->object($object1Name)->exists()); | ||
| $this->assertEquals(!$deleteSourceObjects, $bucket->object($object2Name)->exists()); |
There was a problem hiding this comment.
Using assertEquals with a boolean value is a PHPUnit anti-pattern. It is more idiomatic and readable to use assertTrue or assertFalse directly, which also aligns with the assertion style used elsewhere in this test file.
if ($deleteSourceObjects) {
$this->assertFalse($bucket->object($object1Name)->exists());
$this->assertFalse($bucket->object($object2Name)->exists());
} else {
$this->assertTrue($bucket->object($object1Name)->exists());
$this->assertTrue($bucket->object($object2Name)->exists());
}
There was a problem hiding this comment.
The rest of this file uses assertTrue() and assertFalse() when checking $object->exists(). Should we update this test case for consistency?
There was a problem hiding this comment.
Updated the test case to use assertTrue() and assertFalse() for consistency and readability.
product-auto-label
Bot
added
api: storage
Removed trailing whitespaces in storage/test/ObjectsTest.php which caused the styles GitHub Actions check to fail. [Generated-by: AI]
| $this->assertEquals(!$deleteSourceObjects, $bucket->object($object1Name)->exists()); | ||
| $this->assertEquals(!$deleteSourceObjects, $bucket->object($object2Name)->exists()); |
There was a problem hiding this comment.
The rest of this file uses assertTrue() and assertFalse() when checking $object->exists(). Should we update this test case for consistency?
2 participants
Updates compose file sample to support deleting source objects optionally. Fixes b/441557254
[Generated-by: AI]