Following up on this suggestion before looking into using the command for the Gutenberg work I'm doing at the moment.
⚠️ Apologies for the tab vs spaces changes. I'll try to fix them before merging.
WIP / RFC
I removed the CACHE_KEY computation logic if the parameter was not passed because it was never used. What do you think? I don't think this change would require a major version bump because:
We can say with confidence that it never run because 1) all the callers in this plugin pass a cache and 2) because of the u flag in the shebang, the line CACHE_KEY=$2 would have failed the script if no value had been passed, therefore never running the fallback logic. Moreover, the script uses $3 and $4 later on, further proving that $2 was always, implicitly, required.
So, while technically making $2 required when it previously wasn't should count as a breaking change, in practice $2 was always required and I think we shouldn't worry with that.
However... Since we are taking the time to add options it might be beneficial to use them for both the path to archive and the key to use. That's would make the command more robust, as it would be order agnostic. save_cache --path PATH_TO_CACHE --key KEY works just as well as save_cache --key KEY --path PATH_TO_CACHE but save_cache PATH_TO_CACHE KEY and save_cache KEY PATH_TO_CACHE produce different results. If we were to do that, then it would definitely count as a breaking change.
What do you think?
[ ] I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.
Following up on this suggestion before looking into using the command for the Gutenberg work I'm doing at the moment.
⚠️ Apologies for the tab vs spaces changes. I'll try to fix them before merging.WIP / RFC
I removed the
CACHE_KEY
computation logic if the parameter was not passed because it was never used. What do you think? I don't think this change would require a major version bump because:So, while technically making
$2
required when it previously wasn't should count as a breaking change, in practice$2
was always required and I think we shouldn't worry with that.However... Since we are taking the time to add options it might be beneficial to use them for both the path to archive and the key to use. That's would make the command more robust, as it would be order agnostic.
save_cache --path PATH_TO_CACHE --key KEY
works just as well assave_cache --key KEY --path PATH_TO_CACHE
butsave_cache PATH_TO_CACHE KEY
andsave_cache KEY PATH_TO_CACHE
produce different results. If we were to do that, then it would definitely count as a breaking change.What do you think?
CHANGELOG.md
if necessary.