Skip to content

Comments

Combine config reader and writer into single config class#77

Closed
shadowhand wants to merge 1 commit intokohana:3.3/developfrom
ushahidi:3.3-fixed-config-database
Closed

Combine config reader and writer into single config class#77
shadowhand wants to merge 1 commit intokohana:3.3/developfrom
ushahidi:3.3-fixed-config-database

Conversation

@shadowhand
Copy link
Contributor

Enables proper extensibility.

@shadowhand
Copy link
Contributor Author

Honestly, not sure how much this matters, since Ohanzee.DB already fixed all this.

@enov
Copy link
Contributor

enov commented Oct 1, 2014

@shadowhand, for minor versions, we have promised a drop-in upgrade. You did not even care to delete Config_Database_Reader and Config_Database_Writer classes. You have changed the internal as well as public API, breaking applications that rely on the deleted classes.

How about changing Kohana_Config_Database_Writer into Config_Database_Writer on the line below?
https://github.com/kohana/database/blob/3.3/develop/classes/Kohana/Config/Database.php#L12
Doesn't that make things easier to extend, the Kohana way? I guess that was what the original author intended to do anyways... ;)

I am sorry boss, but this is a 👎 for me.

Cheers!

@shadowhand
Copy link
Contributor Author

Problem is... you can't possibly extend database config with the current separation between reader/writer. But again, I am not going to put any more effort into this, because it has all been corrected in the Ohanzee version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants