ChangeSet Validator (was Re: Harvesting classes without class comments?!)

Doug Way dway at riskmetrics.com
Fri Nov 7 04:18:01 UTC 2003


On Thursday, November 6, 2003, at 02:17 PM, Brent Vukmer wrote:

> Thanks, Scott!  Wow.  Who knew?  I didn't :)  But then, I'm ignorant
> about *tons* of cool stuff already built into the image.
>
> I have a feeling that we ought to always run those five checks as part
> of the "mail to list" ChangeSorter option.  Also might be good to
> incorporate in the BFAV and in Doug's UpdateTool. Hmm.....

Yeah, somehow I didn't know about these checks either. :)  Well 
actually, I've used "check for slips" before, but that's it.

I've been thinking about creating a changeset "validator" tool to 
handle this sort of thing.  Having it happen as part of the "mail to 
list" makes the most sense to me.  It could do a bunch of checks and 
only allow a changeset to be posted (for possible inclusion) if:

- it includes a preamble
- any new classes include a class comment (like Scott's check)
- all methods have author initials timestamps
- methods contain no linefeeds
- the changeset name doesn't start with a number, and that it ends with 
a dash + initials
- the changeset name is less than 28 characters (well, maybe.  We've 
been requiring this to fully support Mac OS 9, which may not really be 
necessary)
- maybe a few other checks, eventually maybe we could also check for 
egregious SLint violations

If any of these checks fail, it would tell you what to do to resolve 
the problem.  The idea would be that only submissions which have 
already passed these checks would be considered for inclusion.  (Maybe 
the changeset, or the "mail to list" email, could be somehow tagged 
that it had passed the checks... hm.)  I'm not sure we'd want to force 
these checks on people merely filing out changesets, though.  People 
should still be allowed to throw any old code up to the mailing list if 
they don't care about inclusion in the base release.

We should only include checks for things that we really want to 
enforce... I don't think we want to enforce that all methods have 
comments, for example.  Checking for "slips" might be good, except on 
certain occasions it is reasonable to output to the transcript.

Also, it might be good to have the validator operate on an external 
file (like the FileContentsBrowser and ConflictChecker do) rather than 
an already-loaded ChangeSet object.  At least, it's probably easier to 
file out an in-image changeset and validate it externally (without 
mucking up your image), than vice versa.

And then yeah, we could also run the validator when incorporating, 
although if it's always done when people post to the list, it should 
very rarely fail. :)

- Doug


> Cheers,
> Brent
>
> P.S. Documentation folks, this would be a good thing to put on the 
> wiki.
>
> -----Original Message-----
> From: Scott Wallace [mailto:scott.wallace at squeakland.org]
> Sent: Thursday, November 06, 2003 2:01 PM
> To: The general-purpose Squeak developerslist
> Subject: Re: Harvesting classes without class comments?!
>
>
> Most squeakers are probably familiar with the simple 
> change-set-checking
> tools that have been available via the "Dual Change Sorter" for years,
> but for those who are not...
>
> When preparing a change-set for publication, and also when vetting
> someone else's change-set for harvesting, you can point a
> DualChangeSorter at the change-set, and then bring up (and *keep up*
> with the push-pin) the "more..." branch of the change-set-list menu:
>
> The five "check for" items allow you to explore potential deficiencies
> in the change-set; they detect "slips" (e.g. halts and Transcript
> references,) messages that have no senders, uncommented methods,
> uncommented classes, and uncategorized methods.
>
> If it were agreed that no change-set would be considered a candidate 
> for
> harvesting until it had passed all these checks, it would help.
>
>




More information about the Squeak-dev mailing list