Refactor RubyFileWriter #222
Conversation
There was a problem hiding this comment.
Thanks @cllns! These changes all look good to me 👍🏼
Particularly in support for the adjustment to the #initialize/#call args for the FileWriter. This is much more consistent with how we design classes in general.
I left you a couple of questions, but they're minor, so happy for you to merge when ready.
| end | ||
| super(name: normalized_name, slice: slice, **opts) | ||
| def call(name:, **opts) | ||
| name = "#{inflector.singularize(name)}_repo" unless name.end_with?("_repo") |
There was a problem hiding this comment.
Just checking: by reassigning name here, that new value will still be passed up to the superclass method via the no-args version of super below?
There was a problem hiding this comment.
Yup! Which I wasn't expecting. I wrote it explicitly as super(name: name, **opts), but my Rubocop autocorrect got rid of it since it's equivalent to super.
| normalized_name = inflector.underscore(name) | ||
| ensure_valid_name(normalized_name) | ||
| def call(namespace:, key:, base_path:, **_opts) | ||
| _namespace = namespace |
There was a problem hiding this comment.
What's the reason for this extra assignment? It doesn't look like we use _namespace anywhere?
There was a problem hiding this comment.
Ah, it was to appease Rubocop, but clearly the better move is to just remove the namespace: arg altogether, which is what I just committed.
Addresses #202
Although, I ended up not using
slice.source_path. I had an implementation working that just took aHanami::Sliceand used.namespaceand.source_pathbut it proved hard to test. I couldn't figure out how to register a slice into theTest::App.Instead, the RubyFileWriter takes
namespaceandbase_path, and we do the mapping inGenerate::Command.Happy to change this over if we can figure out a simple way to test that, to simplify the code, but I also feel like it's reasonable that a "File Writer" would require a namespace and a path as separate args, even though they're coupled in our apps.
I also changed the RubyFileWriter implementation so that
calltakes the file-specific args, instead of no args forcalland all of them being passed to the initializer. I implemented this via helper class, as passing around all the variables that need to be passed around proved to make the code very messy.I want to handle this refactoring before using RubyFileWriter in other generators.