Principle of minimal invasion in codebases you do not know

This is part of the Semicolon&Sons Code Diary - consisting of lessons learned on the job. You're in the workflows category.

Last Updated: 2024-11-23

When changing the Project B PHP code to switch over to my s3 bucket, I also deleted various lines throughout the codebase explicitly specifying the bucket, believing - without testing - that setting a default bucket in the config would "just work" based on some documentation for some other version of Laravel.

This turned out not to be the case, and this caused me production bugs and much extra work and confusion down the road.

<?php

$s3->putObject(array(
    'Key'        => $folder.'/'.$randomString.'.'.$extension,
    // I deleted the following line all over the place under the false
    // assumption that this would be automatically filled out.
    'Bucket' => "hardcoded-bucket-name,"
    'SourceFile' => $new_filename,
    'ACL'        => 'public-read'
));

What I should have done in this "rescue mission/no maintenance budget" project was just change the hard-coded bucket name - i.e. to do the minimal necessary to get the job done. This reduces the risks in a codebase I do not know with a version of a framework I have never used.

Perhaps if I foresaw future change, I could have been ever so slightly more invasive and use an ENV variable, but that's it:

e.g.

<?php

$s3->putObject(array(
    'Key'        => $folder.'/'.$randomString.'.'.$extension,
    // I deleted the following line all over the place
    'Bucket' => "ABC"getenv('S3_BUCKET'),
    'SourceFile' => $new_filename,
    'ACL'        => 'public-read'
));

Lesson

Change as a little as possible, especially in codebases you are not familiar with. By being conservative and avoiding refactoring or "cleaning up" at the same time, you reduce errors.