Ok, thank you,  otherwise it seems to fine, may I ask you to commit it
to the trunk with that correction?

--Hannes

On 1/22/13, Bert Freudenberg <bert at freudenbergs.de> wrote:
>
> On 22.01.2013, at 21:57, commits at source.squeak.org wrote:
>
>> A new version of Monticello was added to project The Inbox:
>> http://source.squeak.org/inbox/Monticello-hjh.529.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Monticello-hjh.529
>> Author: hjh
>> Time: 22 January 2013, 9:56:58.248 pm
>> UUID: 4fa5f98e-0261-6240-ac33-d98759df6a77
>> Ancestors: Monticello-nice.528
>>
>> Added comments to MCDefinition, MCPackage and MCSnapshot.
>>
>> A MCDefinition is a model for Smalltalk code. A package snapshot
>> (MCSnapshot) is a collection of these definitions.
>>
>> MCPackage uses PackageInfo to find out which methods and classes belong to
>> a package.
>>
>> =============== Diff against Monticello-nice.528 ===============
>>
>> Item was changed:
>>  Object subclass: #MCDefinition
>>  	instanceVariableNames: ''
>>  	classVariableNames: ''
>>  	poolDictionaries: ''
>>  	category: 'Monticello-Base'!
>>  MCDefinition class
>>  	instanceVariableNames: 'instances'!
>> +
>> + !MCDefinition commentStamp: 'hjh 1/22/2013 21:38' prior: 0!
>> + A MCDefinition is a model for Smalltalk code. A package snapshot
>> (MCSnapshot) is a collection of these definitions.
>
> Up to here it's fine, but ...
>
>> + What does
>> +     MCDefinition clearInstances.
>> + do?
>> +
>> + It nils out the quick-access cache to its subinstances. #allInstances is
>> very slow as it needs to scan the whole object memory, whereas retrieving
>> an instance from the "instances" WeakSet is O(1).
>> +
>> + Why can this be done safely?
>> +
>> + Because re-using a definition is only a space optimization.
>> +
>> + What is the impact of doing this?
>> +
>> + When loading/creating another snapshot, the definitions that are in both
>> snapshots will not be shared but occupy space twice.
>> + !
>
> ... this does not belong in the class comment. The discussion showed that
> calling clearInstances is not a good idea. If at all, an explanation could
> be added as a method comment, but please not in Q/A format.
>
>> + !MCPackage commentStamp: 'hjh 1/22/2013 21:38' prior: 0!
>> + MCPackage uses PackageInfo to find out which methods and classes belong
>> to a package.
>> +
>> + More see MCDefinition.
>> +
>> + !
>
> The "More" ref does not seem useful to me.
>
>> Item was changed:
>>  Object subclass: #MCSnapshot
>>  	instanceVariableNames: 'definitions'
>>  	classVariableNames: ''
>>  	poolDictionaries: ''
>>  	category: 'Monticello-Base'!
>> +
>> + !MCSnapshot commentStamp: 'hjh 1/22/2013 21:40' prior: 0!
>> + A MCSnapshot is a snapshot of an MCPackage. It is a collection of
>> MCDefinitions.
>> +
>> + See also MCDefinition.!
>
>
> Similarly, the "See also" ref is unnecessary. If someone wants to know about
> MCDefinition, they would browse that class, they do not need an extra hint
> to do so.
>
> - Bert -
>
>
>
>