Applying update to generators #30
Conversation
There was a problem hiding this comment.
A brief description for the PR would be nice to have a better understanding as I had to spend a few cycles to understand the high-level idea first.
I understand some refactoring is happening here with overall code improvements which is great 👍 and left a couple of comments.
| fi | ||
| } | ||
|
|
||
| function checkBucket() { |
There was a problem hiding this comment.
It's nice to have a function here 👍
let's also follow the more common bash shellcheck naming standards (similar to python, lowercase with underscore separators):
| function checkBucket() { | |
| # a brief function description | |
| function check_bucket_name() { |
Could we also include a line comment for this function describing briefly what it does?
|
|
||
| GITHUB_IDENTIFIER="$(echo $($GITHUB_ACTION_PATH/operations/_scripts/generate/generate_identifier.sh) | tr '[:upper:]' '[:lower:]')" | ||
|
|
||
| function generateIdentifiers () { |
There was a problem hiding this comment.
Same as for the other function, naming and description would be nice to haves.
And quickly looking at it, I didn't understand what kind of Identifiers the function generates.
generate_bucket_names() or generate_bucket_identifiers perhaps would be more clear?
|
Gotcha - Sorry about that @armab, this was work that was ported over from an upgrade to the docker-to-ec2. (Hence why I was pushing for a commons last week) I'll post up some cleanup shortly |
Because of the dynamic name generation different length identifiers need to be created to created. For example, a continuously used state bucket is the
X-tfnamed bucket, if aX-logsbucket needs to be created as well, then a separate identifier needs to be created for it.This merges all common identifier generations into a single file to clean up repeat files.