[squeak-dev] Re: DoIts in Deltas - give us feedback please

Göran Krampe goran at krampe.se
Wed Sep 9 08:50:26 UTC 2009


Hi Andreas!

Andreas Raab wrote:
[SNIP of stuff we agree on]
> However, in point #2 below (i.e., about the doIt "it will NOT be 
> executed when the Delta is applied") I think you're being overly 
> cautious which will only lead to confusion. What is the point of 
> recording a doIt it if it's not applied? I cannot think of a case where 
> I'd like to cover a doIt and not apply it - if I have such an entity I 
> call it a comment, not a doIt and this distinction should be explicit.

Mmm, my point was probably that if you set a Delta to record doIts and 
then just file it out without looking at it you will be bringing lots of 
garbage along...

So the whole idea was to prevent such "accidents" by making sure that 
the developer marks the doIts (a menu choice or such).

One simpler approach may be to just throw up a warning when one is about 
to "distribute" a Delta (in whichever way that may be, outside the 
image) that it indeed includes x number of doIts, proceed or cancel.

> In other words, if I mean to provide a doIt for information purposes 
> then I should add this as a comment instead of a doIt since it has no 
> implication on the delta stream. But if I record (or add) a doIt then I 
> *mean* to execute it, otherwise I'd use a comment. Having a doIt that's 
> also sort-of-a-comment will only cause confusion in the long term. Be 
> clear about the purpose of comments (you do have comments, don't you?) 
> and things will work out fine.

Hehe, comments... yeah, we do since the Delta itself and also each 
Change has a "properties" dictionary which can contain all Tirade proper 
keys/values. So adding a #comment property with accessors should be 
trivial, thanks for reminding me. This should also be done to cover the 
preamble using some naming convention.

> BTW, you say in your things to explain list that the doIt was marked "to 
> be run on apply using #executeOnApply:. This is an EXPLICIT step for a 
> reason." But you haven't actually provided a reasoning for it, just said 
> that the developer needs to do so and honestly I fail to see it :-)

Again, the idea was to prevent doIts to "slip in" without the developer 
doing it consciously. Perhaps a mere "warning" as described above would 
be sufficient?

> Oh, and I think that marking a doIt as "dangerous" in the UI is silly. A 
> doIt is just as dangerous as any other bit of code. Just mark it as a 
> doIt, that is code executed directly when loaded[*] and it'll be fine.
> 
> [*] Funny, I only noticed when re-reading that my very *definition* of a 
> doIt is a bit of code that's executed directly when loaded so the whole 
> "executeOnApply" bit in my book is really a contradiction for something 
> called a doIt.

Mmmmm, not sure what you mean here [*] - a Delta is first deserialized 
into the image and then applied. Ahh... you meant that it would be odd 
to include doIts that are not executable on apply? Yes, that would 
indeed be odd UNLESS you take into account that a Delta can do double 
duty as a true log.

Also, I meant "dangerous" as in "makes the reversability unpredictable". 
If you do not have doIts then Deltas can actually assure you 
reversability (unless there is a bug of course).

> Lastly, class initialization. Same rules apply as above, just be 
> predictable.

Yes, that is very important.

 > I don't think you'll get around to supporting class
> initialization but the tricky part here is the ordering when you really 
> want it to be reversible. You may have to support out-of-order 
> anti-doIts that is if you have a series of changes followed by a doIt, 
> you must reverse all the preceding changes before applying the anti-doIt.

Humptidum... well, I wasn't going to use doIts for class initialization!

> FWIW, I don't think the ability to reverse doIts and class 
> initialization is relevant in any practical context. Nobody's cared 
> about that with Monticello at all and things still work quite all right. 
> Just be predictable (he sez for one last time ;-)

So to sum it up, how about this then:

1. If we record doIts they are NOT marked to executeOnApply. To prevent 
accidental replaying of a log. You can then use the UI, select such a 
doIt and just use a menu choice to mark it as executeOnApply if you like to.

2. If you *add* a doIt using the UI for Deltas (which probably will be 
the reasonable way to add doIts anyway, just like with 
preamble/postscripts today) then the default will be executeOnApply = 
true. A second pane will be available for the antiDoit.

3. Skip the warning on distribution, better to make sure the UI shows 
that a Delta includes doIts that has executeOnApply = true. Fair?

4. Let the applier contain the class initialization logic. We do not add 
any Changes specifically for that. And let's make it predictable. I 
still would like to see a proposal for that logic. AFAICT it is a bit 
tricksy, especially regarding class ivars and inherited class side 
#initialize.

regards, Göran




More information about the Squeak-dev mailing list