zikula / core

Zikula Core Framework
GNU Lesser General Public License v3.0
238 stars 66 forks source link

work on eliminating some BC Breaks #1856

Closed craigh closed 9 years ago

craigh commented 10 years ago

refs #1853

There seems to be some possibility to remove BC breaks in the areas that are currently in the documentation. I suggest we work on seeking and implementing BC where possible.

note: there is still some question as to whether the old ->getFiles() method might be able to maintain BC or to find another method of providing for BC in this case. (refs #1002, #1261) "one could replace the ParameterBag with a proxy class and magic methods for example. The request object is supposed to be created only once in the front controller btw"

$this->request->filter

note: There may still be some effort to provide a BC layer here. (refs #1261) "if someone cares enough it might be possible to handle with a proxy"

Paginate (Doctrine Extensions)

note: is this a true BC-break?

Events

Could the old events be triggered by the new events for BC purposes only?

craigh commented 9 years ago

events were added back in #2050

craigh commented 9 years ago

@cmfcmf @Guite can either of you explain what the issue is with Paginate (Doctrine Extensions) ? What happened, what was required to be changed and why?

Guite commented 9 years ago

Doctrine 2.2 introduced a Paginator component: http://doctrine-orm.readthedocs.org/en/latest/tutorials/pagination.html

In the 1.3 core we included Doctrine 2.1.4 https://github.com/zikula/core/blob/1.3/src/plugins/Doctrine/Plugin.php#L59 so we had to use an extension for this: https://github.com/beberlei/DoctrineExtensions#paginator

More information with regards to the different implementations can be found here: https://github.com/Guite/MostGenerator/blob/master/org.zikula.modulestudio.generator/src/org/zikula/modulestudio/generator/cartridges/zclassic/models/Repository.xtend#L203 https://github.com/Guite/MostGenerator/blob/master/org.zikula.modulestudio.generator/src/org/zikula/modulestudio/generator/cartridges/zclassic/models/Repository.xtend#L732

craigh commented 9 years ago

@Guite what about including the Extension in 1.4.0 so both options are available?

Guite commented 9 years ago

I wouldn't do this. First the repo isn't maintained:

Warning: This repository is not really maintained anymore. The important paginatior and large collections code was moved to the Doctrine2 core. For all the other extensions, be aware that I don't maintain them anymore.

Second how many 1.3 modules are using it actually? MOST-based modules can be regenerated with ease anyway, so they don't really count.

Guite commented 9 years ago

Also we don't know whether the extension works with Doctrine 2.2+. So if we would really want to provide a BC layer, we would probably need to include the old Doctrine version, too - which is going to become ugly if we need to have two Doctrine installations in parallel.

craigh commented 9 years ago

I was just trying to find ways to keep BC as much as possible. Even though it isn't maintained, we might include it but only if we can confirm it is compatible with D2.2+ (if it is not used, all the better :smile: ).

I agree we should not include two versions of doctrine.

craigh commented 9 years ago

:laughing: :laughing: :laughing:

The extension is _still included_ already :exclamation:

cmfcmf commented 9 years ago

I think I re-included it after discussion with @drak. You should find a PR.

craigh commented 9 years ago

IMO, including it is enough to clear the BC issue.

craigh commented 9 years ago

original ticket is #890. PR is #891.

craigh commented 9 years ago

I've updated the docs for a more accurate representation of that concern in my pending PR.

If anyone is aware of any other vendor library that is also deprecated, then we should also add it to the docs.

IMO when my PR is merged, this ticket is complete.

craigh commented 9 years ago

closed by #2214