Re: Harvester status for Marcus Denker

[what is important to review]
I find myself surprised, not completely unpleasently, at how different
our tendencies are on this.

I say this is a little good, because it's a more complete picture of
what we should be doing than any one of us would have. But it's mostly
bad, because IMO, having each of us checking what we feel like stinks.

A. It sends out a really confused message as to what code should be like
(you need a class comment, except if dvf is checking it, in which case
just make sure you're not adding anything to Object! ;-)
B. It's not a way to improve the quality of the code. Let's face it, if
three of the harvesters don't have the same standards, why the heck
should we assume that Ted does have (and if does, who's does he have?
Mine? Doug's?)

We need a policy. It doesn't have to be complicated, but it needs to
keep the image clean, systematically, not by chance. We need to balance
this with keeping things fun, sure, so we'll have to be smart about it,
but we need to deal with this. If we are warm and fluffy on this one, we
can stop the cleanup projects right now, because quality will go down,
not up, no matter how hard they work.

If we agree on this, here are some things I think we could do -
* Find a good online reference for writing good Smalltalk code, ideally,
something like Kents "Best Smalltalk practices". 
* All of us read it, read all code before approving stuff, and point
people at the manual when said stuff stinks.
* Require use of SLint. It's easy to install, there's a "SmallLint
Tutorial" on SM.
* Maybe make class comments mandatory, and implement some automated
check for this, so I don't forget it.

What do you think?
Other ideas?

Daniel

Doug Way <[email protected]> wrote:
> [email protected] wrote:
> 
> >Daniel Vainsencher <[email protected]> wrote:
> >  
> >
> >>Except for one thing - I haven't seen him post critical comments on any of the
> >>code he's reviewed, and for anyone that's going to be able to get stuff
> >>into the image where we all have to live with it, I want to know that he
> >>can be point out where code is lacking.
> >>    
> >>
> 
> True, although I have to admit that I don't often have comments about 
> the source code that I'm reviewing.  More often I have a comment about 
> some aspect of what the change does. (such as my extra comments about 
> the BoundsInHaloFix2 changeset)
> 
> I guess I usually just give the source code (diffs) a quick look-over to 
> make sure that nothing jumps out at me as being ugly.  As far as judging 
> source code, another factor is who wrote it... typically I will have a 
> level of trust that somone like Ted Kaehler or Ned Konz won't need to 
> have their code heavily scrutinized, but I may look more closely at the 
> code of someone I don't know.  (However, external testing is another 
> matter... that's equally important for all submissions.)
> 
> >Personally I have found that the
> >code I have reviewed from others generally is good - but class comments
> >and method comments tend to lack IMHO.
> >
> >I really want the quality of comments to be higher in the official
> >classes. I would personally never let classes in without class comments.
> >  
> >
> 
> This is probably a good rule of thumb.  New classes should certainly 
> include class comments before we approve them.  It's harder to have any 
> simple rule about method comments... most methods do not need comments 
> and should not have them, but some methods do need comments.
> 
> >>Except for the selection aspect, this is also important because posting
> >>these public comments is the way knowledge is transmitted about good/bad
> >>practice, and not just to the author of the specific code.
> >>
> >>What does everybody think?
> >>    
> >>
> >
> >I definitely think Marcus should be a Harvester - if he likes to be one.
> >:-) 
> >
> 
> I agree.  (Oh wait, I see he's already approving items anyway... :-) )
> 
> - Doug Way
> 
> 
> _______________________________________________
> Squeakfoundation mailing list
> [email protected]
> http://lists.squeakfoundation.org/listinfo/squeakfoundation
From [email protected] Tue Jun 10 10:07:31 2003
Return-Path: <[email protected]>
Delivered-To: [email protected]
Received: (qmail 28440 invoked from network); 10 Jun 2003 10:07:30 -0000
Received: from ns.bluefish.se (HELO leia.bluefish.se) (213.212.22.234)
  by mail.theinternetone.net with SMTP; 10 Jun 2003 10:07:30 -0000
Received: from [213.80.63.110] (helo=aSqueakSystem)
	by leia.bluefish.se with smtp (Exim 3.12 #1 (Debian))
	id 19Pg2M-0000Ts-01
	for <[email protected]>;
	Tue, 10 Jun 2003 12:07:30 +0200
X-Mailer: Celeste 2.0.4917
Date: Tue, 10 Jun 2003 12:07:05 +0100 
In-reply-to: <[email protected]>
Subject: Re: [Squeakfoundation]Harvester status for Marcus Denker
To: Discussing the Squeak Foundation
	<[email protected]>
References: <[email protected]>
From: [email protected]
Message-Id: <[email protected]>
X-BeenThere: [email protected]
X-Mailman-Version: 2.1
Precedence: list
Reply-To: Discussing the Squeak Foundation
	<[email protected]>
List-Id: Discussing the Squeak Foundation
 <squeakfoundation.lists.squeakfoundation.org>
List-Unsubscribe: <http://lists.squeakfoundation.org/listinfo/squeakfoundation>,
	<mailto:[email protected]?subject=unsubscribe>
List-Archive: <http://lists.squeakfoundation.org/pipermail/squeakfoundation>
List-Post: <mailto:[email protected]>
List-Help: <mailto:[email protected]?subject=help>
List-Subscribe: <http://lists.squeakfoundation.org/listinfo/squeakfoundation>,
	<mailto:[email protected]?subject=subscribe>
X-List-Received-Date: Tue, 10 Jun 2003 10:07:31 -0000

Hi all!

Daniel Vainsencher <[email protected]> wrote:
> [what is important to review]
> I find myself surprised, not completely unpleasently, at how different
> our tendencies are on this.
> 
> I say this is a little good, because it's a more complete picture of
> what we should be doing than any one of us would have. But it's mostly
> bad, because IMO, having each of us checking what we feel like stinks.

What we feel like?
 
> A. It sends out a really confused message as to what code should be like
> (you need a class comment, except if dvf is checking it, in which case
> just make sure you're not adding anything to Object! ;-)

I never said that the only thing I check for is a class comment - I just
noted that the code I have been looking at sofar has mostly been lacking
in that department. I would be very wary of letting new methods into
class Object too.

> B. It's not a way to improve the quality of the code. Let's face it, if
> three of the harvesters don't have the same standards, why the heck
> should we assume that Ted does have (and if does, who's does he have?
> Mine? Doug's?)

I am not sure we have such different standards - but I still agree that
we need a policy.

> We need a policy. It doesn't have to be complicated, but it needs to
> keep the image clean, systematically, not by chance. We need to balance
> this with keeping things fun, sure, so we'll have to be smart about it,
> but we need to deal with this. If we are warm and fluffy on this one, we
> can stop the cleanup projects right now, because quality will go down,
> not up, no matter how hard they work.
> 
> If we agree on this, here are some things I think we could do -
> * Find a good online reference for writing good Smalltalk code, ideally,
> something like Kents "Best Smalltalk practices". 
> * All of us read it, read all code before approving stuff, and point
> people at the manual when said stuff stinks.

What did you mean with "read all code" - you mean instead of simply
"trusting" someone? I always read all code I review, regardless of
author.

> * Require use of SLint. It's easy to install, there's a "SmallLint
> Tutorial" on SM.
> * Maybe make class comments mandatory, and implement some automated
> check for this, so I don't forget it.

I really think class comments should be mandatory.

> What do you think?
> Other ideas?

Well, it would be interesting to write down the policy somewhere - at
least the non obvious and the mandatory stuff. I am not saying "Swiki"
because I am a bit tired of simply creating more and more pages on the
Swikis, but if no other smarter way is found then sure, why not.

regards, Göran
From [email protected] Tue Jun 10 10:36:18 2003
Return-Path: <[email protected]>
Delivered-To: [email protected]
Received: (qmail 9776 invoked from network); 10 Jun 2003 10:36:18 -0000
Received: from iramx1.ira.uni-karlsruhe.de (141.3.10.80)
  by mail.theinternetone.net with SMTP; 10 Jun 2003 10:36:18 -0000
Received: from acb7d210.ipt.aol.com ([172.183.210.16]
	helo=i09nbmarcus.ira.uka.de)
	by iramx1.ira.uni-karlsruhe.de with asmtp (Exim 3.30 #10 (Debian))
	id 19PgUD-0005tI-00; Tue, 10 Jun 2003 12:36:18 +0200
Received: from marcus by i09nbmarcus.ira.uka.de with local (Exim 3.33 #2
	(Debian))	id 19PgOh-0001q6-00; Tue, 10 Jun 2003 12:30:35 +0200
Date: Tue, 10 Jun 2003 12:30:34 +0200
From: Marcus Denker <[email protected]>
To: Discussing the Squeak Foundation
	<[email protected]>
Subject: Re: [Squeakfoundation]Harvester status for Marcus Denker
Message-ID: <[email protected]>
References: <[email protected]>
	<[email protected]> <[email protected]>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <[email protected]>
User-Agent: Mutt/1.4.1i
X-BeenThere: [email protected]
X-Mailman-Version: 2.1
Precedence: list
Reply-To: Discussing the Squeak Foundation
	<[email protected]>
List-Id: Discussing the Squeak Foundation
 <squeakfoundation.lists.squeakfoundation.org>
List-Unsubscribe: <http://lists.squeakfoundation.org/listinfo/squeakfoundation>,
	<mailto:[email protected]?subject=unsubscribe>
List-Archive: <http://lists.squeakfoundation.org/pipermail/squeakfoundation>
List-Post: <mailto:[email protected]>
List-Help: <mailto:[email protected]?subject=help>
List-Subscribe: <http://lists.squeakfoundation.org/listinfo/squeakfoundation>,
	<mailto:[email protected]?subject=subscribe>
X-List-Received-Date: Tue, 10 Jun 2003 10:36:19 -0000

On Mon, Jun 09, 2003 at 12:37:11PM -0400, Doug Way wrote:
> >I definitely think Marcus should be a Harvester - if he likes to be one.
> >:-) 
> >
> 
> I agree.  (Oh wait, I see he's already approving items anyway... :-) )
> 
Sorry ;-) somewhat premature, I guess. But those were really simple:
a one-liner and two documentation-only, if I remember correctly.

Regarding Daniel's suggestion to comment more: Yes, you are right, I'l
write more comments about the code in future posts. 

[email protected] wrote:
> I would personally never let classes in without class comments.

There's actually a test in the Tests-package for that: I've done
a Abstract testing-class called "ClassTestCase". 
If you inherit from this one instead of TestCase (and name the test
"<name of the class>Test", you get an error if the class-comment is empty.

(and you get 3 more tests: testNew, testUnCategorizedMethods, testRecompileAll)

There are not yet tests for every class in the system, but as soon as the
testserver is running, I'l add more. 

        Marcus

-- 
Marcus Denker [email protected]  -- Squeak! http://squeak.de