On 08/25/2013 02:58 PM, David Deutsch wrote:
>
> Space after/before brackets
>
> 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.
Yes. We are consistent in this through our code base, which means all of
as (read: I and Thomas ;)) prefer no spaces here. So, we'd like to keep
this and your commits will be much smaller.
> 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:
I just find better to read:
}
else if (true) {
something();
}
else {
something2();
}
than
} else if (true) {
something();
} else {
someting2();
}
However, I saw Thomas uses the later.
> Is there anything else that you find needs discussion or was the rest OK?
I don't like for/foreach in few lines, as I saw somewhere in your links:
foreach(
array(
elem1, elem2, elem3,...
) as $var
) {
> 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?
No. As you might notice we are using branches and git-master is in
(active) development mostly all the time. I don't know, maybe we
shouldn't do such changes before 1.0 release.
--
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
[email protected]
http://lists.roundcube.net/mailman/listinfo/dev
