ezpublish-legacy
ezpublish-legacy copied to clipboard
ezsystems
EZP-21146 remove includes
There are still a lot of include that should be triggered by php autoload.
This PR is meant to remove some of them.
These seems to be breaking changes, legacy is supposed to stay (very) stable.
Why BC ? The code only uses php autoload where php4 needed include pattern (made with settings here).
With that PR, we would finally be able to override some code without hacking the kernel...
kernel override is in essence a clean way to hack the kernel, but maybe I'm wrong regarding the BC. @patrickallaert you see any possible BC breaks here?
@patrickallaert you see any possible BC breaks here?
I haven't seen any BC break.
+1 from me, especially since we are at the beginning of the cycle of next release.
I would however commit this as a single commit by rebasing the PR or by merging manually.
@patrickallaert you're right this is better with all commits rebased. There is no need to have so much commit for the same purpose.
Ok, only remaining issue seems that the ini change seems to indicate the settings are deprecated, while they are remove right? If so they need to be mentioned in doc/bc as removed and why. If not so a note in doc/bc about deprecation is still needed.
Just one remark from me: this PR is mixing code change and CS fixes. If I'm all for fixing CS issues, I rather prefer keeping things separated as it makes reviewing and browsing the code history much simpler.
@patrickallaert I removed as you said the code i previously commented.
I also reverted the file ezdatatype.php which didn't have any use in this PR.
I think you should remove any cosmetic changes, because a good practice is to not mix functional and cosmetic changes in the same commit.
I mean that your commit https://github.com/tharkun/ezpublish-legacy/commit/ef0b68b0c1b5f7c6c23832b693c94ddeabb9cd67 contains cosmetic changes, i.e. space removal, mixed with functional changes.
These should be separated.