Closed flash-gordon closed 5 years ago
timriley
commented
5 years ago @flash-gordon I'm not sure this actually needs to wait for https://github.com/dry-rb/dry-configurable/pull/59 to be merged? The code I'm reverting there never made it out into a gem release, so this dry-container change should just be able to work with the already-released version of dry-configurable.
timriley
commented
5 years ago Apart from that, this looks good to me.
timriley
commented
5 years ago Ah, there's one problem here:
Which came as part of your work on https://github.com/dry-rb/dry-system/pull/97, will need to be adjusted, since that's expecting namespace_separator to be a class attribute 😬
solnic
commented
5 years ago Wait. Why do we have to use dry-configurable here?
timriley
commented
5 years ago Oh nm, I see you addressed it in https://github.com/dry-rb/dry-system/pull/103
timriley
commented
5 years ago @solnic because dry-container originally used dry-configurable and it makes no sense to cause API churn (this was only changed to use class attributes when we went down the dead-end path of making dry-configurable rely on dry-struct).
solnic
commented
5 years ago OK fair enough
timriley
commented
5 years ago @flash-gordon Have tried this + https://github.com/dry-rb/dry-system/pull/103 in my app and it looks good there too.
flash-gordon
commented
5 years ago Nice, gonna merge everything today and cut releases by the end of the week.
Once we revert dry-struct-related changes in https://github.com/dry-rb/dry-configurable/pull/59 we can switch back to dry-configurable. The goal is to make the DSL block-less in https://github.com/dry-rb/dry-configurable/pull/56 and use is as a standard way of configuring dry-container and its descendants.