[RCD] Codebase cleanup, PSR-1/2 compliance - and before you say no, I'm offering to do the work

Ben Kaizer ben at carthage.dk
Sun Aug 25 15:56:09 CEST 2013


Hi David,

We are glad you want to contribute.

You are right, it clearly avoids issues to stick to a standard style. I
personally think the code base would benefit from such a homogenisation.
Then, since almost every experienced developer has their own style, it
is not uncommon to temporarily alter indentation and spacing on the fly
at the editor level and then switch back to conform to the standard when
committing or just saving.

Thanks for putting the time!

Cheers,


On 08/25/2013 01:58 PM, David Deutsch wrote:
> Hi Alec,
>
> Thanks for getting back to me! Alright, understood - some of the style
> you don't like.
>
> ---
>
> Space after/before brackets
>
> Spaces after parenthesis is my personal style. Is that the part you
> object to? In PSR-2, it's only space before parenthesis. I do think
> they make things more readable in some cases, though, for instance,
> compare this:
>
> master:
> https://github.com/roundcube/roundcubemail/blob/master/plugins/archive/archive.php#L225
> cleanup:
> https://gist.github.com/daviddeutsch/6285028#file-archive-php-L262
>
> But again, this is up to taste and it's a setting in an automatic
> formatter, anyhow, so while I have a taste in it, I don't really have
> strong feelings either way.
>
> ---
>
> New line before else/catch
>
> I suppose you mean between brace and statement? PSR-2 is rather
> specific on no newline between brace and statement and I must say I
> agree with that. If an if/else is hard to read, it's usually the
> content between the brace and fanning out the braced is just a
> bandaid. It's mostly a problem with long if/else statements, for instance:
>
> master:
> https://github.com/roundcube/roundcubemail/blob/master/plugins/archive/archive.php#L23
> cleanup: https://gist.github.com/daviddeutsch/6285028#file-archive-php-L23
>
> It's arguable whether one is that much more readable than the other,
> but the important consideration is trying to maintain the same style
> derived from what is the default case. And the default case should be
> concise if/else statements where a newline between braces and
> statement would be weird.
>
> The main issue I had with the current version of the codebase is that
> a LOT of ways to do this are applied - brackets, no brackets, mixture
> of brackets and no brackets (if I recall correctly). That made it a
> bit hard to figure out what was the main style.
>
> ---
>
> Is there anything else that you find needs discussion or was the rest OK?
>
> Test scripts are of course a good idea as well, although I don't think
> I would be competent enough to make good contributions in that area at
> the moment. That's why I suggested cleanup - because once a style is
> decided, it's sort of mindless chomp-through work.
>
> Another note - since large codebase changes like this one can be a
> problem in terms of getting clean merges during active work in other
> areas, it is usually preferable to do them when there isn't that much
> activity. Since 0.9.3 was just released - would at the moment be a
> good time?
>
> cheers,
> David
>
>
> On Sun, Aug 25, 2013 at 12:36 PM, A.L.E.C <alec at alec.pl
> <mailto:alec at alec.pl>> wrote:
>
>     On 08/20/2013 09:15 PM, David Deutsch wrote:
>     > So - in a nutshell, that's what I'm offering to do for Roundcube.
>
>     In general we accept such contribution.
>
>     > I'd be happy to accommodate to special customs, of course,
>     although I do
>     > think that sticking to PSR - flawed as it may be in spots -
>     really helps
>     > enable collaboration by convention. Let me know if this sounds like
>     > something that would at all be welcome in this project.
>
>     I don't like some of your style, precisely, space after/before
>     brackets.
>     I also prefer a new line before else/catch.
>
>     If you want to learn Roundcube code, consider also creation of
>     more test
>     scripts.
>
>     --
>     Aleksander 'A.L.E.C' Machniak
>     LAN Management System Developer [http://lms.org.pl]
>     Roundcube Webmail Developer  [http://roundcube.net]
>     ---------------------------------------------------
>     PGP: 19359DC1 @@ GG: 2275252 @@ WWW: http://alec.pl
>     _______________________________________________
>     Roundcube Development discussion mailing list
>     dev at lists.roundcube.net <mailto:dev at lists.roundcube.net>
>     http://lists.roundcube.net/mailman/listinfo/dev
>
>
>
>
> _______________________________________________
> Roundcube Development discussion mailing list
> dev at lists.roundcube.net
> http://lists.roundcube.net/mailman/listinfo/dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.roundcube.net/pipermail/dev/attachments/20130825/ff581634/attachment-0001.html>


More information about the dev mailing list