search

oridoki / koriko

Deploy system
MIT License
4 stars 1 forks source link

Helpers & other dependencies managed by the DIC #8

Closed kpacha closed 11 years ago

kpacha commented 11 years ago

Delegating the management of the logger, the ssh helper and the default set of task helpers,

Adding a new default helper is as easy as adding a new key at the KorikoContainer. For future extensions, the container is also customizable/extensible.

This KorikoContainer is just a Pimple container with the default components as a logger, a ssh driver or the MySQL helper. If the isolation between task helpers and the other components is a requirement, the helpers could be encapsulated inside another container embeded in the main one.

The cost of this improvement is the 'init' call before running the app (see the diff at tests or the bin/koriko file)

The 'pimple' branch is merged with your 'introducing_actions'.

kpacha commented 11 years ago

Also, this closes Issue #1, doesn't it?

adriacidre commented 11 years ago

Hey, you're dragging my commits, I'm not sure if they are stable / tests ok, I will review it later. Also this PR can't be automagically merged ;-)

kpacha commented 11 years ago

You are right!

Now it's merged with the dummy_implementations branch but I've found a 'little' problem with your helper implementation: the constructor of the HelperAbstract requires a task to be injected... and this Pimple simple extension is not supporting this. So I've removed the _initTaskHelpers() method from the KorikoContainer class with the helper folder.

kpacha commented 11 years ago

The dummy_implementations branch exposed the loadCommands method. So we already had to trigger the command loading before running the app if the default command folder was disabled.

Now, as we need to ensure the DIC is setted up before loading any command, I thought the easiest and cleanest way was mantaining the.loadCommands method as protected and exposing this init one chaining the required steps.