Hi Thomas,
Thanks for chiming in!
- PSR-2 compliance
I generally agree to using PSR-2 even if there are some rules that personally I don't like. Especially the if/else newlines.
Spaces after parenthesis is definitely a no-go!
Alright, good to have an agreement on that.
- I will ruthlessly convert instances of yoda conditions.
I veto against this as we (almost) consequentially write condition statements in yoda style. Ain't broken but will highly confuse the core developers who work with the codebase every day.
Alright, I can live with that. Although I'm not so sure that it would actually confuse developers.
- /program/localization - mostly just indents
You should not touch the /program/localization/*/*.inc files as they're automatically generated by Transifex but I agree to modify index.inc.
Ah, that makes sense.
- PEAR vs. composer
Roundcube still supports PHP 5.2 and thus Composer is not (yet) an option. Just ignore that for the moment. Be assured that as soon as we drop support for PHP < 5.3 we'll fully switch to Composer.
Makes sense as well. It will definitely be exciting to move this codebase to 5.3 since a lot of it is already pretty modular and can benefit from composer and 5.3 features like namespaces etc..
- /installer
The installer generally needs a refactoring with proper MVC separation and localized texts. Feel free to dig into this.
This will be a particular delight :-) For now, I will only clean up things, but as I test RCM more thoroughly, I might end up installing it a lot and then being annoyed at and wanting to change the installer. So good to know you're open to change there ;-)
- Skins
Yes, the images are run through pngcrush. Actually I'm using http://imageoptim.com which uses different optimization algorithms.
Ah, ok. I just tested three files through pngcrush on the commandline and they came out slightly smaller. Not a big thing for now and probably something that should be automated anyhow. Then again, until you get to automation, it might be that you've already been taken by the new Flat UI craze and switched to an icon font anyhow ;-)
The templating system of Roundcube was created before the all the popular template engines raised. Switching to twig or handlebars would be nice but requires proper planning and a sustainable path to keep backwards compatibility with all the existing skins out there. Please just ignore that for now.
Yeah, it did smell a lot like technical debt from way back when. I was relieved to find that at least you're not doing anything crazy like XSLT (oh the things I've seen...)
twig and handlebars are definitely good options. I particularly like handlebars (and have contributed to handlebars-php https://github.com/XaminProject/handlebars.php/pulls?direction=desc&page...) because it is a syntax that is in itself independent of PHP and thus highly portable, which is usually a good idea if you want to have other developers help out. Of course twig is a more "native" solution, so it's not a bad choice either.
And sure, fully agree that it's nothing for right now. Definitely something to keep in mind for a 2.0 version of RCM.
LESS would definitely be an option for CSS. Unfortunately none of as experience with it and no time to dig into that (I admit being one of the very old school web developers from the last century). For releases I'd prefer to compile the less files and deliver static CSS files. That however adds node to the dependency chain of Roundcube.
Also definitely something for a 2.0 branch, yes.
And yes - those can be compiled before delivery. Although I think with the plugin structure, it could be interesting to have a master CSS file compiled on the fly, which is very, very trivial using lessphp - https://github.com/leafo/lessphp - The good news with this is that the library is _very_ portable as it has no dependencies whatsoever. You basically just hand it a list of less files and it gives you a CSS file back. Minified, even, if you want.
I've worked with LESS for quite a while now and it does help manage pretty much all of the annoyances of day-to-day CSS use. I've given examples on HN before which might help illustrate that: https://news.ycombinator.com/item?id=5370482 - and then there are things like managing colors as variables in one place etc. etc.
I could go on ;-)
I have to mention that personally I don't force such refactorings as they make it more difficult to track (functional) changes in the SCM. But I can see the overall benefit and I very much appreciate your efforts to contribute to this.
Thank you. From my own codebases, I know how hard it is and I very much appreciate how hard it is to accept this for an established codebase such as this one. But I also know that sometimes, the best thing that can happen is if somebody else comes along and just nudges you along the way. As developers, we like to stick with what we built and know, but then something big is changed and you end up not really sure why you liked it so much the way it was before ;-)
In general, I think I have enough agreement for now to start on the first couple of things on the list. I will update with a ping here when I have a couple of PRs to show here and we can discuss the finer details of me actually doing stuff.
cheers, David