Refactor parsers

[AydinHassan]

I refactored the parsers for fun - to make a little more modular and improve the PathTranslation stuff a little, I would prefer this to go in to a develop branch as it'll probably need a bit of testing (If this patch is actually wanted, I realised I changed quite a bit).

What this PR does:

• Removes the dependency on PathTranslationParser from all the Deploy Strategies
• Change PathTranslationParser to a simple decorator of original strategy
• Move Parsers to there own namespace
• Add tests for PackageXmlParser and fix some bugs with not imported exceptions
• Remove some redundant code in the strategies to make them a little simpler
• Abstract the Parser choosing to a Factory class
• Add some test assets for parsers
• Simplify unit-tests, especially the PathTranslationParser ones

If this is too much I can split it up or if you plain don't want it, that's fine :) I guess I was just trying to learn some stuff!

If you do want it, i'll squash it before you merge

[davidverholen]

I recently thought about taking the parsers from https://github.com/bragento/bragento-composer-installer (which are basically the ones from the hackathon installer ;)) to an own package. Maybe would be a good idea, to use a shared composer library for them since they always need to do exactly the same for magento 1??

[AydinHassan]

Yeah I think that's a good idea. Same for deploy strategy file system stuff. I wanted to mess about with some prototypes and really don't want to rewrite all that logic as it's pretty flaky, so would be nice to just pull it in.

[Flyingmana]

I already started to move some stuff into own libraries here https://github.com/Cotya/Deploy-helper

[Flyingmana]

btw. I like to have multiple commits, if they are like in this PR :)

But as this is some heavy refactoring, maybe its better to put this into the 3.0 branch. As hint, I removed the core-installer in the 3.0 branch, as it does not work very good with a lot of features already implemented for modules. I would prefer to improve the deploy of the magento core packages (mage-all-latest and the others) to solve the problem the core installer did via a workaround.

[AydinHassan]

I'm fine with this going in 3.0 branch, I can rebase it at some point. I'm away for couple of days now, going to climb some mountains 🌁🌌

Also +1 for removing/refactoring the core installer, I think we should concentrate on getting the module stuff really solid and maybe provide the core stuff via a separate package ? Or just use mine 😬 haha.

Sent from my iPhone

On 27 Dec 2014, at 01:25, Daniel Fahlke notifications@github.com wrote:

btw. I like to have multiple commits, if they are like in this PR :)

But as this is some heavy refactoring, maybe its better to put this into the 3.0 branch. As hint, I removed the core-installer in the 3.0 branch, as it does not work very good with a lot of features already implemented for modules. I would prefer to improve the deploy of the magento core packages (mage-all-latest and the others) to solve the problem the core installer did via a workaround.

— Reply to this email directly or view it on GitHub.

[davidverholen]

the core just needs to be deployed with files only (resolving directories), that would solve most Problems. (I think thats what you meant @Flyingmana ?)

One way would be to have every File listed in the mapping (you had the modman generator, right @AydinHassan ?), but I would rather change the behavior of the Installer itself, so that the parsers already resolve all mappings down to files and then persist them for example in json files. That way very many bugs and problems where just solved ;)

Moving the Core Installer to a separated package sounds good. And then use common libraries for the critical tasks like parsing and mapping with high test coverage could also save a lot of headaches.

[Flyingmana]

seems we all agree on this :)

I think one possible way would be, to add a parsing step to the modman file, which expands the definitions to file leven, resolving all the globs and directories. This should work good together with the removing on file level, you did already good work for :+1:

[davidverholen]

i'm already spamming in the library ;) When this works good, the Installer should not be a big Problem anymore.

the step to resolve all files should then also be in the abstract class, as would be needed, at least for the composer mapping, too. I'm not sure but i think the package.xml also has not any file mapped?

[Flyingmana]

the package.xml does list every single file mapped

[AydinHassan]

closing in favor of #172.