Hey guys, I went ahead with making the plan and put up a gist here: https://gist.github.com/daviddeutsch/6376013
Let me know what you think! I would estimate that this will take a week or two at most. The larger blocks will be one or two PRs per day, smaller ones maybe 6 or 7. Didn't find a good way to tackle doing lesser updated directories first, but I think the splitting up of the PRs will help reduce the pain of merge conflicts. Although, of course, that will also depend a tiny bit on how fast you merge ;-) cheers, David On Sun, Aug 25, 2013 at 7:39 PM, David Deutsch <[email protected]> wrote: > 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 <[email protected]> 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 >> [email protected] >> http://lists.roundcube.net/mailman/listinfo/dev >> > >
_______________________________________________ Roundcube Development discussion mailing list [email protected] http://lists.roundcube.net/mailman/listinfo/dev
