KCP looking for external reviewers
danielv at netvision.net.il
Sat Apr 5 00:39:45 UTC 2003
Stephane Ducasse <ducasse at iam.unibe.ch> wrote:
> Hi daniel
> On Friday, April 4, 2003, at 06:50 PM, Daniel Vainsencher wrote:
> > Cool. I want you to get reviewers, but "Kernel" sounds a little scary.
> No it is not. In fact Morph is far more complex. This is just that this
> is a bit meta.
I agree completely. I think the most of the kinds of changes you're
making don't touch the meta aspects at all, or even serious
implementation complexity. However, for others to be aware of this, and
therefore consider themselves maybe capable of reviewing the changes, it
has to be said clearly.
> Yes this is goal one.
> Having a small kernel based on layers. So that we could use the kernel
> without the layers.
I note the web page now notes this more clearly. As you start focusing
on specific problems, make it clear, and identify which changes are
related to which problems.
> > For example, is self environment always a good
> > replacement for Smalltalk? external reviewers shouldn't need to reverse
> > engineer your changes, just review them.
> Hardcoding Smalltalk everywhere is simply bad because the method
> existed before. This way a class can define its own environment.
> Hardcoding everything hampered that.
Wonderful - this and other design decision should be written somewhere
people can read it before diving into the code.
> > I don't understand what the changeset
> > is supposed to do. This prevents me from finding something nice and
> > reviewing it.
> But there is always a premable (or this was a mistake from us). We
> editing the page because everything is in the preamble. So we were
> going too slow.
> Tell me if the information in the preamble is not enough.
If you think about the process, you'll see that it's important that the
documentation be on the web page itself. Probably reviewers will go
through one changeset each time, and they want to be able to choose the
ones that interest them, or that they understand. A little more trouble
for you, much clearer for the reviewer.
For the same visibility reasons, the policy is that when someone adds a
QA tag, his name/initials be written near it, and that the details be
included in the mail. The same logic applies to the web page, so please
try to make these things visible in the future.
About reviewing itself - I've reviewed changes 4,7 and 9 and will
support them when you send them in. In change 11, you implement
Behavior>>allUnsentMessages by referencing the current environment,
instead of Smalltalk. Why not remove it completely? what's your
rationale for leaving functionality in the kernel, or moving it out,
into something else?
Is SystemDictionary part of the Kernel? if it is, the kernel will
continue to reference many things for a long time... maybe it's easier
to extract it's core functionality into a new class and refactor the
leftovers later? if it isn't, you shouldn't refer to it from Behavior.
More information about the Squeak-dev