Hi Jakob,


thanks for the detailed review! :-)

<http://www.hpi.de/>

> How about using MCDefinition>>#isRevisionOf: instead of a description = b description? Saves people like me from wondering why description is used here (and what it contains anyway).

Because I did not know this small but useful method. :P Will be fixed in the next version ...

> In domain terms, why is it significant here that the definitions are members of an organization? Why does this procedure not apply to organization changes or script changes, for example?

Fair question. #description is only unique for a definition that is part of an organization. Given a set of two MCVersions one of which would add and the second one would delete an MCOrganizationDefinition for separate packages, by the current implementation these organization definitions would be revisions of each other and thus would be wiped up in #forgetSuperfluousMethodRemovals. However, since you're commenting this, I'm asking myself two questions:

  1.  Is it a valid operation to remove an entire organization definition under any circumstances?
  2.  Couldn't we instead modify the #definition implementation of MCOrganizationDefinition and insert the containing system category name at this place? Semantically, this would look better for me, but unfortunately, MCOrganizationDefinition does not have a relevant instance variable ...

> Instead of allocating another Collection instance, you could also use an OrderedCollection for the removals and use removeAllSuchThat:, or put all the conditions in just one reject block.

Good idea.

> > Thanks to Tom Beckmann (TB)
>
> Finally, doesn't Tom use tobe as his author initials?

Does he? In Pheno (2018), he uses TB. However, the following returns to 'tobe':

SystemNavigation authors at: 'Tom Beckmann'

Hm ... ;-)

Best,
Christoph
________________________________
Von: Jakob Reschke <forums.jakob at resfarm.de>
Gesendet: Montag, 16. November 2020 17:28:19
An: squeak-dev at lists.squeakfoundation.org; Thiede, Christoph; Tom Beckmann
Betreff: Re: [squeak-dev] The Inbox: Monticello-ct.732.mcz

Hi,

Am Mo., 16. Nov. 2020 um 16:00 Uhr schrieb <commits at source.squeak.org>:
>
> +       removals := removals reject: [:removal |
> +               removal isOrganizationMember and: [
> +                       additions anySatisfy: [:addition |
> +                               addition isOrganizationMember and: [addition description = removal description]]]].!

How about using MCDefinition>>#isRevisionOf: instead of a description
= b description? Saves people like me from wondering why description
is used here (and what it contains anyway).

In domain terms, why is it significant here that the definitions are
members of an organization? Why does this procedure not apply to
organization changes or script changes, for example?

Instead of allocating another Collection instance, you could also use
an OrderedCollection for the removals and use removeAllSuchThat:, or
put all the conditions in just one reject block.

> Thanks to Tom Beckmann (TB)

Finally, doesn't Tom use tobe as his author initials?

Kind regards,
Jakob
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20201116/70093909/attachment.html>