Folks,

Sometimes I really don't know whether to laugh or to cry when it comes to 
the random eye-ball principle of reviewing fixes.

Lex wrote (in an entirely unrelated thread):
> Anyway, I agree on the change to the last line, that #true should be
> true.  Doh!  Someone should do an image scan for every true, false, or
> nil that is in a literal array....

The change that we're looking at here is was made in update 
5736nilTrueFalseInLiteralArray-avi which resulted from a discussion about 
whether nil, true and false in literal arrays should be compiled as symbols 
or not. While I don't mind the decision being made that these should be 
compiled to the literal objects instead of the symbols it really drives me 
crazy that not a single of the reviewers of that change to a low-level, 
high-impact area (we're talking about Scanner here mind you; this is even 
lower-level than a syntax change!) spent half a second to think about what 
this change means to the existing system, e.g., what might break because of 
it.

It's not hard to find out you know - simply browsing the "senders" of nil, 
true and false and checking where they are used in the existing system gives 
you a perfect idea what parts might be affected. I will show you a few here:

ClassBuilder>>reservedNames
Player>>acceptableScriptNameFrom:forScriptCurrentlyNamed:
StandardScriptingSystem>>acceptableSlotNameFrom:forSlotCurrentlyNamed:asSlotNameIn:world:
ColorType>>updatingTileForTarget:partName:getter:setter:
DataType>>updatingTileForTarget:partName:getter:setter:
EToyVectorVocabulary>>eToyVectorTable
ExternalData>>fields
Win32Handle>>fields
X11Drawable>>fields
X11GC>>fields

In other words, if you think about the implications for two seconds and just 
briefly scan over the affected places you can see that the change has a good 
chance of breaking just about THREE major parts of Squeak: Recompilation of 
all classes, eToys, FFI. And this doesn't even include the parts it already 
potentially DID break, such as everything in external packages.

It seems to me that changes with such major implications ought to be a 
little more carefully looked at than by just say "(et) works for me". I am 
MAJORLY pissed off that we start doing these random eye-ball reviews in 
parts of Squeak that will affect its suitability for everyone - how posting 
a GC, or method lookup changes and have a few people who understand next to 
nothing about the VM and the implications of such a change say "works for 
me"?

I am *really* pissed off. I am pissed off to the point that I am proposing a 
change in the current reviewing and approval policies in a way that certain 
areas (which coincidentally include compiler and vm) can ONLY be reviewed 
and approved by people who have shown their qualification in this area. NOT 
by Joe Blow "works for me (et)".

Regards,
  - Andreas