> On 05 Feb 2015, at 17:46, Ben Coman <[email protected]> wrote:
> 
> 
> On Thu, Feb 5, 2015 at 11:12 PM, GitHub <[email protected] 
> <mailto:[email protected]>> wrote:
> 
>   Log Message:
>   -----------
>   40477
> 
> 14669 Delay refactoring (part 2a) - avoid UI locking up when timer event loop 
> is stopped
>         https://pharo.fogbugz.com/f/cases/14669 
> <https://pharo.fogbugz.com/f/cases/14669>
> 
> 
> 
>  Thanks Max and Marcus for review and pushing this through.  
> 
> Next step is Issue 14353 "Delay refactoring (Part 2) - change from 
> milliseconds to microseconds" and I started to review rebasing this onto the 
> latest build.  Now left over from Part 1 
> (https://pharo.fogbugz.com/default.asp?14261 
> <https://pharo.fogbugz.com/default.asp?14261>) is lots of conditional code 
> wrap of the form...
> 
> self newCodeEnabled 
>   ifTrue: [
>       "my new code"
>       ]
>  ifFalse: [
>       "the old code"
>       ]
> 
> However this conditional code wrap clutters things making it harder to review 
> changes going forward. So I was wondering it was preferable for me to clean 
> that out first, before pushing 14353 for review.  For Part 1 it served a few 
> purposes...
> 
> (1) It was required to make an instantaneous transition to the new 
> architecture.  However this no longer required since integration of Issue 
> 14669 now allows the DelayScheduler to be stopped to change code without 
> locking the system. 
> 
> (2) It provided an emergency fallback to disable the new architecture, since 
> this was deep-diving into critical infrastructure. However Part 1 was 
> integrated in November, so is that long enough for the code to have proved 
> itself?   

I would say so, yes.

> 
> (3) It allowed easy switching back and forth for benchmarking.  Maybe still 
> useful, but I don't think this is enough on its own to keep the 
> newCodeEnabled conditional code wraps.  Benchmarking can still be done on 
> separate build before and after integration of subsequent changes.

You can recompile on the fly if you really need to use the same image.

> 
> Please advise how you'd like me to proceed.
> cheers -ben
> 
> 

I’d go ahead and remove the conditionals and the old code. The release of Pharo 
4 is planned for April I think, so even if something goes wrong, there’s still 
time to fix it. Removing the clutter will also make it easier to review your 
changes; and it will make writing code for you easier.

Pharo 4 is still in development (unless I missed something), and development 
means unstable environment. People who use Pharo 4 are aware of that.

That’s my two cents anyway.


Keep up the great work!

Cheers,
Max

Reply via email to