[squeak-dev] The Trunk: Monticello-bf.509.mcz

Chris Muller asqueaker at gmail.com
Thu May 31 18:39:03 UTC 2012


> 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


More information about the Squeak-dev mailing list