Closed ghost closed 8 years ago
@samsonasik @acelaya Would you mind doing a review?
Sometimes I'm amazed by my stupidity and the code I write...
Bitbucket is down and travis has failed installing PhantomJS.
usually won't take long, hopefully on your next commit, the bitbucket service back again ;)
Warm regards,
Abdul Malik Ikhsan
Pada 21 Okt 2015, pukul 17.27, Stanimir Dimitrov notifications@github.com menulis:
Bitbucket is down and travis has failed installing PhantomJS.
— Reply to this email directly or view it on GitHub.
I implemented the module in my CMS and it took me 5 mins for the whole process. There the module loads for ~300ms. Nothing more to add. Ready for merge in no one has anything to say.
@acelaya could you do final review before it got merged?
I'm not probably going to be able to review it until weekend. Is it ok?
Alejandro Celaya Alastrué El 22/10/2015 15:27, "Abdul Malik Ikhsan" notifications@github.com escribió:
@acelaya https://github.com/acelaya could you do final review before it got merged?
— Reply to this email directly or view it on GitHub https://github.com/sitrunlab/LearnZF2/pull/158#issuecomment-150223119.
Ok ;), thank you ;)
Warm regards,
Abdul Malik Ikhsan
Pada 22 Okt 2015, pukul 20.28, Alejandro Celaya notifications@github.com menulis:
I'm not probably going to be able to review it until weekend. Is it ok?
Alejandro Celaya Alastrué El 22/10/2015 15:27, "Abdul Malik Ikhsan" notifications@github.com escribió:
@acelaya https://github.com/acelaya could you do final review before it got merged?
— Reply to this email directly or view it on GitHub https://github.com/sitrunlab/LearnZF2/pull/158#issuecomment-150223119.
— Reply to this email directly or view it on GitHub.
@Stanimirdim92 you're missing migration command to add this module to sql that will be generated in data/DoctrineORMModule/Migrations
merged after rebase at https://github.com/sitrunlab/LearnZF2/commit/db4af03b712ee274893c6969b2c5d0a6e98c397d ;)
@Stanimirdim92 the module is life now http://learnzf2.sitrun-tech.com/learn-zf2-themes , thanks ;) @acelaya if you have a chance to take a look, and found any issue, feel free to PR ;), thanks ;)
Theme switcher is working now, but you have to manually refresh the page to change th text.
TODO:
unit testsFix some code bugs, better cohesion and couplingMove getConfig() logic to a factory classAdd styles for all themesSimplifyMerge both modules into one