The Trunk: Monticello-bf.509.mcz

> On Tue, May 29, 2012 at 3:07 AM,  <[hidden email]> wrote:
>> Bert Freudenberg uploaded a new version of Monticello to project The Trunk:
>> http://source.squeak.org/trunk/Monticello-bf.509.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Monticello-bf.509
>> Author: bf
>> Time: 29 May 2012, 12:06:58.832 pm
>> UUID: 02bc3aaf-6de3-4b06-8264-852ba776aeb4
>> Ancestors: Monticello-bf.508
>>
>> Allow to specify branch name in MCFileBasedRepository>>versionNamesForPackageNamed:
>>
>> =============== Diff against Monticello-bf.508 ===============
>
> Bert, there is a problem with this change.  If you open the repository
> up, there is NO selected package,

>> Here's a question for cmm:
>>
>> While looking at your MCFileRepositoryInspector code it seems to me that versionNames isn't even used anymore, but only allVersionNames. Indeed, when I remove the super send in MCFileRepositoryInspector>>initializeVersionNames and instead add #refreshEmphasis, everything appears to work fine. So, could the super send be removed for good?
>
> Hi.  One of the side-goals of the MCRepository upgrade was to unify
> the RepositoryInspectors to work generically with any MCRepository.
> This was initially started in Monticello-cmm.423 and completed by
> Monticello-cmm.425 with the removal of MCFileRepositoryInspector in
> its entirety.
>
> But because FileBasedRepository's can only access the entire directory
> contents to do any kind of interrogation of the versions present, it
> became wasteful to do that under the new sustainable API, which only
> requires to know the versions of the *selected* package.
>
> Therefore, an entirely new MCFileRepositoryInspector subclass was
> created in Monticello-cmm.427 with its cache of 'allVersionNames'.
> Now, it could execute its own #versionNamesForSelectedPackage without
> having to re-get all the directory entries.  MCFileRepositoryInspector
> is unwanted, but tolerated solely for the improved performance.
>
> So, let's please not inch our way toward more dependency on
> 'allVersionNames'.  We may not have any senders of versionNames from a
> MCFileRepositoryInspector today but it is fair game to be called in
> the future since it is part of the standard API for the
> MCRepositoryInspector.
>
>> If we could remove it, then my new nil check in MCRepositoryInspector>>initializeVersionNames wouldn't be necessary anymore ...
>
> We should not remove the super send, but I think your nil guard is
> appropriate in any case.  Oh, BTW, Array empty is better than Array
> new because it uses a single canonicalized empty Array which can be
> shared throughout the system.  I don't think Squeak's MC is portable
> to other Smalltalks as it is, so it should be ok to use its niceties.
>
>> Also, I assume the reason for introducing allVersionNames was to avoid asking the repository on every package list click. However, wouldn't it have been simpler to do in MCRepositoryInspector, not MCFileRepositoryInspector? Right now we have a weird mix where the versionNames variable is unused etc.
>
> #allVersionNames is unsustainable and has no business anywhere but in
> FileBased[Repository|Inspector]'s.  versionNames is absolutely used
> for inspectors of any other type of MCRepository than the FileBased.
>
> - Chris