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=1&sort=created&state=closed)
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
_______________________________________________
Roundcube Development discussion mailing list
[email protected]
http://lists.roundcube.net/mailman/listinfo/dev

Reply via email to