ellisv / oxid-console

[UNMAINTAINED] OXID Console is php console application for OXID Shop. It provides an API for writing various commands. We have shipped very common for every oxid shop project commands out of the box.
MIT License
23 stars 17 forks source link

fixes 2 findings in fix:states command: #30

Closed keywan-ghadami-oxid closed 8 years ago

keywan-ghadami-oxid commented 8 years ago
  1. fix:states command of this version oxid console 1.1. resets the the order of modules extention chain 'aModules'
  2. fix states was kind of slow because it saves the 'aModules' even if nothing changed
ellisv commented 8 years ago

Good job! Thanks for contribution.

One little thing that I am concerned about is if someone has built a custom solution to build their module chain relying on this. What I mean is:

$moduleIdsChain = $repo->loadModuleIds();
$module = oxNew('oxStateFixerModule');

foreach ($moduleIdsChain as $moduleId) {    
    $module->load($moduleId);
    $module->fixExtend(); // or even fix()
}

So this would modify module chain in the order of specified module ids (and I know at least one project which does it like so).

So I can not release a patch which breaks this functionality. But I think this is a really nice to have anyway, what we could do is create new method something like fixExtendWithoutRemoval and make fix:states command use that instead.

So I'll merge your pull request and I will work on keeping the backwards compatibility (:

ellisv commented 8 years ago

Alright, I have already worked on this a bit. You can see the changes here: https://github.com/EllisV/oxid-console/compare/v1.1.6...ba667c742a7138e410c208396970b14c8d1dda6a

You ok with that?

ellisv commented 8 years ago

There has been already a v1.1.7 release with your change. You can check the changelog at https://github.com/EllisV/oxid-console/blob/v1.1.7/CHANGELOG.md

keywan-ghadami commented 8 years ago

sorry for answering so late, am on holiday - so am writing from my private account. I agree with you that this is kind of bc break, i was not aware that other projects may relay on this behaviour, but they should note that i did not see this behaviour in the 2.x version of the oxid console. Your change is therefore very good. I am thinking about unifying the oxid console versions so that tools using the oxid console do not have to care about the version. Thank you for maintaining the oxid console!!! best regards