[KCP][ENH] KCP-0160-SystemChangeNotif (and related changesets -
[et] mostly works (found 1 error), [er] I like it)
ducasse
ducasse at iam.unibe.ch
Thu Jan 29 08:12:18 UTC 2004
On 28 janv. 04, at 23:11, Avi Bryant wrote:
> 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.
no this is an old comment
The version two (KCP-0170-CompilerProtocolRefactoring.4.cs ) changes
SystemNavigation default forgetDoIts into Smalltalk forgetDoIts.
>
> 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?).
It is a client but we tight it together to have the same behavior as
now.
>
> 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>
>
>
More information about the Squeak-dev
mailing list
|