I'm posting a review on this thread, but in fact I tested several 
related changesets.  Doug, if you need this separated out let me know, 
but it's easier to review them as a group.

Starting from 3.7a #5629, I loaded the following changesets, in order:

KCP-0135-PrepareMovingChangesLog.cs
KCP-0136-MovingChangesLog.cs
KCP-0137-CleanUtilities.cs
KCP-0147-moveUtilitiesEvaluateToCompiler.cs
KCP-0148-moveBrowseUncommentedMethodToSystemNavigation.cs
KCP-0160-SystemChangeNotif.cs
KCP-0165-SystemChangeNotifHooks.2.cs
KCP-0170-CompilerProtocolRefactoring.2.cs

I tested various kinds of compiling, renaming, and removing, without 
running into any errors, and checked to make sure that the changeset 
recorded them properly.  This all seemed to work fine - I don't think 
we're losing any changeset behavior we used to have.

I did run into one problem when filing in an external file - at the end 
of PositionableStream>>fileInAnnouncing: is "SystemNavigation default 
forgetDoIts".  This causes a DNU.  Since it's also marked with "self 
flag: #ThisMethodShouldNotBeThere", I wonder if someone simply forgot 
to remove it.

I read over the code for all of the changesets involved.  I don't have 
intimate knowledge of the parts of the system involved, but as a user 
of the affected protocols I was pleased with the changes.  The 
SystemChangeNotification code will make Monticello's change tracking 
*much* simpler - I had to resort to some ugly hacks in the past that 
will no longer be necessary.  I also like the compiler protocol 
refactoring.  For example, returning a compound CompiledMethodWithNode 
object from #basicCompile:notifying:trailer:ifFail:  is easier to 
understand and use than the somewhat baroque 
#compile:notifying:trailer:ifFalse:elseSetSelectorAndNode: interface it 
replaces.  I'm also happy to have the installation of the method into 
the method dictionary better separated from the compilation of the 
CompiledMethod, and to have the notification as part of 
#addSelector:withMethod: rather than as part of compilation.  One thing 
that might be nice would be to further separate this out so that adding 
the source to the .changes file wasn't done at the same time as 
installing the method into the method dictionary (or could the .changes 
file just be another client of the notifications?).

Another small note - it concerns me that #addSelector:withMethod: is no 
longer sent by the normal compilation mechanisms, since people may have 
overridden this - they would now need to override 
#addSelector:withMethod:notifying:.  I don't know what the solution 
should be, this may just be acceptable breakage.

Avi

On Jan 28, 2004, at 6:59 AM, n.schaerli at gmx.net wrote:

> from preamble:
>
> "Change Set:		KCP-0160-SystemChangeNotif
> Date:			28 January 2004
> Author:			Roel Wuyts & Nathanael Schaerli
>
> New version of the system change notification framework. Working with
> image v5623."!
> <KCP-0160-SystemChangeNotif.cs.gz>