On 22.01.2013, at 14:10, "H. Hirzel" <hannes.hirzel at gmail.com> wrote:

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

It's simpler if you submit it again.

- Bert -



> 
> 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 -
>> 
>> 
>> 
>> 
>