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 <[email protected]> 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 <[email protected]> wrote:
>
>>
>> On Aug 31, 2013, at 2:00 PM, David Deutsch <[email protected]> wrote:
>>
>> Another possibly neat comparison:
>>
>> Original:
>> https://github.com/roundcube/roundcubemail/blob/master/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php#L206
>> Cleanup:
>> https://github.com/daviddeutsch/roundcubemail/blob/37167c5ce1c00cb4f42b7f59a9ff56b81b3cd874/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php#L219
>>
>> 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
>> [email protected]
>> http://lists.roundcube.net/mailman/listinfo/dev
>>
>
>
_______________________________________________
Roundcube Development discussion mailing list
[email protected]
http://lists.roundcube.net/mailman/listinfo/dev

Reply via email to