Hey, it seems like we're halfway there to you guys agreeing on this. Sweet! ;-)
all of us (read: I and Thomas ;)) prefer no spaces here
Sure, no problem.
However, I saw Thomas uses the later.
Well, since I and PSR-2 agree with Thomas here, maybe it's a good idea to go with that? ;-)
I don't like for/foreach in few lines
I think you're talking about the RedBeanPHP stuff I linked, right? (Couldn't find any instance in the Roundcube examples I sent you, but I'm tired, so I might have missed something...) In that case, it was accepted policy in that code, so I went with it. I generally like it in some instances - again, when it helps understanding the code and if it serves staying under the 80 character limit. I was originally hesitant myself, but nowadays, I quite like it, it saves up on generated variables, too. Well, maybe this is something for later ;-)
No. As you might notice we are using branches and git-master is in
(active) development mostly all the time
Yes, I did see that, which is why I posted the question - I just wasn't sure whether it is "very" active right now since I have no frame of reference.
I don't know, maybe we shouldn't do such changes before 1.0 release.
Quite honestly, that is a recipe for "never" ;-) Not saying that there will never be a 1.0 release, but that pushing codebase cleanup usually begets further pushing of codebase cleanups. It is your choice, of course, all I can do is offer my help. But I actually think a good cleanup before a 1.0 is a very good thing since you'll likely be maintaining 1.0 code for quite a while (legacy setups etc.), so having that clean and syntactically close to the repository is usually a good way to avoid confusion.
Maybe we can do it like this: I would set a schedule of how I would iterate through the codebase and make individual pull requests for major directory blocks (say a couple hundred or low-thousand changes per PR). I would try to set it up in such a way that I touch the most frequently touched areas last. That way, you can already test drive my cleanup in low-interest areas and aren't interrupted as much by me meddling with stuff you're often working on. Of course, I would put up the list here first, so you can veto or change it, since I very probably will miss some obvious cues.
In general, the process will be two- or three-passed. First pass is just basic formatting (largely automatic), second pass is in-depth stuff like proper PHPdoc blocks (partly automatic), third pass is actually hunting for bugs or clearing up structures that could be unclear. So the idea with that is that I could roll back to a less controversial commit within a PR and we could already commit that while discussing the stuff that didn't turn out as acceptable.
cheers, David
On Sun, Aug 25, 2013 at 5:58 PM, A.L.E.C alec@alec.pl wrote:
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 dev@lists.roundcube.net http://lists.roundcube.net/mailman/listinfo/dev