PR #7 is in and pretty much done: https://github.com/roundcube/roundcubemail/pull/116
while PR #6 is still being worked on: https://github.com/roundcube/roundcubemail/pull/115
This is one of the larger ones, so it is expected to take longer. I think I did manage to get some stuff simplified already (see earlier message), but I'm still not that satisfied with the result. I also think that the code itself makes poor use of available stuff in RCM to begin with, like the html class or the ::gettext() method in the plugin, so I will probably look into that. Stuff like that is a lot more draining than regular cleanup, though, so I'll probably keep it on the side and do a little every now and then. It would definitely help to have the permission to add methods to that class since there are a LOT of repeated statements. For now, I will keep a look out for existing methods that I can call in.
PR #7 is a good example for something that Thomas worried about earlier - I didn't see a commit coming in concerning a plugin that I was working on two days ago. I did a local merge of those changes and pushed them upstream to the PR, so everything is fine there. Not sure whether that was understood earlier, but it is of course primarily my responsibility to keep up with commits and make sure a merge is straightforward for you guys.
The next couple of days, I won't have as much time for cleanup, so I would appreciate if we could use that brief timeout to discuss the finer points and have some final decisions going forward. For smaller stuff, I could probably do quick adjustments to follow what was decided - that would help maybe accepting the first PR(s) into the codebase. Not to be too pushy here, of course, just would appreciate seeing actual confirmation that the work I'm putting in really will end up going somewhere ;-)
cheers, David
On Sat, Aug 31, 2013 at 10:53 PM, David Deutsch skoremail@gmail.com wrote:
Thanks.
And find them I did!
It's just that Thomas said earlier they were the standard in the codebase and me changing them would confuse existing developers. So I remain unconvinced there is what I'm saying... ;-)
-David
On Sat, Aug 31, 2013 at 10:43 PM, Cor Bosman cor@xs4all.nl wrote:
On Aug 31, 2013, at 2:00 PM, David Deutsch skoremail@gmail.com wrote:
Another possibly neat comparison:
Original: https://github.com/roundcube/roundcubemail/blob/master/plugins/managesieve/l... Cleanup: https://github.com/daviddeutsch/roundcubemail/blob/37167c5ce1c00cb4f42b7f59a...
The code may be /slightly/ less DRY, but, I find, a lot more readable.
Definitely more readable.
I must also say that so far, I find very few yoda conditions! ;-)
Find them you will.
Cor
Roundcube Development discussion mailing list dev@lists.roundcube.net http://lists.roundcube.net/mailman/listinfo/dev