ssm-diff icon indicating copy to clipboard operation
ssm-diff copied to clipboard

Baseline Refactor

Open ambsw-technology opened this issue 5 years ago • 0 comments

Before submitting PRs to add new behaviors, I wanted to wrap my head around the current codebase. I started with some basic contributor-friendly changes:

  • add documentation
  • clarify variable names (e.g. local instead of l)
  • add test cases

I had to do a lot of mocking to get my first wave of test cases working. To reduce the complexity of test cases, I needed to rearrange code to better encapsulate behaviors. This PR is the product of that effort and includes changes like (edit: renamed State classes to clearly indicate storage backend):

  • To ensure consistent behavior through their lifecycle, all classes are now configured on __init__ (should avoid issues like #15 for programmatic users)
  • To support custom diffing classes/behaviors (e.g. issues #11, #13, and #16), DiffResolver (formerly FlatDictDiffer) is now injected into the ParameterStore (formely RemoteState) constructor.
    • To configure DiffResolver without coupling, I created a configure() classmethod that uses functools.partial()` to pre-populate constructor kwargs.
  • All flattening behavior is encapsulated in DiffResolver. This simplifies the interface on ParameterStore and YAMLFile (formely LocalState) which only accept nested dicts (though RemoteState uses some flattened endpoints).
  • Some of the changes (e.g. more git-like method names on DiffResolver) could be reverted, but I changed the signature of most/all methods so it wouldn't really help backwards compatibility.

This PR should be 100% backwards compatible for CLI users. Obviously, the interfaces for most classes/methods have changed so it will be a breaking change (i.e. major release) for programmatic users.

I know big rewrites aren't normally welcomed from a new contributor, but I hope you'll forgive the overhaul since it should simplify/enable additional improvements. I'm going to submit additional PRs, all based on this foundation. If you object to this direction, I can certainly try to rewrite more to your liking.

ambsw-technology avatar Apr 29 '19 20:04 ambsw-technology