Add a "tidy" command to find files that might be missing or unneeded.#305
Add a "tidy" command to find files that might be missing or unneeded.#305
Conversation
mipearson
force-pushed
the
mp-stackmaster-tidy
branch
from
bca0aa6 to
b851d52
Compare
mipearson
force-pushed
the
mp-stackmaster-tidy
branch
from
b851d52 to
66fc41c
Compare
| templates = Set.new(find_templates()) | ||
| parameter_files = Set.new(find_parameter_files()) | ||
|
|
||
| status = @config.stacks.each do |stack_definition| |
There was a problem hiding this comment.
I feel like this could be a separate method like def remove_used(templates, parameter_files) for clarity. (or maybe even one for each?) But that's just nitpicking :)
There was a problem hiding this comment.
Had a go at this but as it's referencing multiple sets (and has side effects where it prints missing templates) it complicates things a bit. Decided it's not worth it for this length of method.
| end | ||
| end | ||
|
|
||
| def rel_path path |
There was a problem hiding this comment.
I think the rest of the code uses parens around parameters.
| parameter_files.each do |path| | ||
| StackMaster.stdout.puts "#{rel_path(path)}: no stack found for this parameter file" | ||
| end | ||
| end |
There was a problem hiding this comment.
Can we return a non-zero exit code if any of them is not empty? That would be nice for CI.
There was a problem hiding this comment.
Thought about it, but they're mostly indications rather than errors - eg in the data platform repositories, there's multiple stack_master.yml files that share a template directory.
They'll both find "extra" files as some templates are only used by one of the two configs .. but they're exceptions rather than the rule.
There was a problem hiding this comment.
I wonder if we should have a flag to enable or disable checking for orphaned templates?
There was a problem hiding this comment.
Maybe - my intention for this wasn't as a CI pass/fail tool but a convenience one, especially as the method for knowing which directories/files are referenced is not 100% reliable.
patrobinson
left a comment
patrobinson
left a comment
There was a problem hiding this comment.
Approved with comments 👍
| parameter_files = Set.new(find_parameter_files()) | ||
|
|
||
| status = @config.stacks.each do |stack_definition| | ||
| parameter_files.subtract(stack_definition.parameter_files) |
There was a problem hiding this comment.
It's ok to have a missing parameter file though right, if the stack takes no parameters or has defaults?
There was a problem hiding this comment.
Oh nevermind,I understand what's going on here now. parameter files - (stacks)
3 participants
Sometimes you delete stacks but don't remove their parameter files or templates.
Sometimes a template is renamed or removed, but stacks still depend on it.
stack_master tidy(I feel like it needs a better name) helps you find these things.