search

dizda / CloudBackupBundle

Be able to backup your database(s) and upload it to the cloud (Dropbox, Amazon S3, GoogleDrive, etc.)
MIT License
196 stars 57 forks source link

Cleanup #88

Closed jongotlin closed 8 years ago

Nyholm commented 8 years ago

:+1: I agree with this change cleanup and I agree with the removal of the getters. But the latter requires a new major release.

If we want this we should mark the functions as @deprecated now and merge this prior next release.

dizda commented 8 years ago

:+1: guys

Nyholm commented 8 years ago

Btw, I just saw this commit https://github.com/dizda/CloudBackupBundle/commit/c05a4fdaf9fc36a6402a6484f977b2dd522d274d

The getters was just added from this PR: https://github.com/dizda/CloudBackupBundle/pull/75 and included in v2.2.0

dizda commented 8 years ago

Thanks for pointing that out! But this, in the future, should be managed through those events, right?

Nyholm commented 8 years ago

Events should just tell the world that something has happened. They should not be able to change the behavior.

dizda commented 8 years ago

Gotcha!

Nyholm commented 8 years ago

This looks good. No BCs any more.

Are we ready to merge this?

Nyholm commented 8 years ago

@dizda and @jongotlin Make a decision about the getters. Are they a good idea? If you want an instance of the services you should use DI.

Im :+1: to do as Jon already done an add a @deprecated notice. @dizda, do you want them to be removed in 4.0?

dizda commented 8 years ago

:+1: Agree, all is said!