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

Andreas Raab andreas.raab at gmx.de
Wed Sep 9 04:20:35 UTC 2009


Hi Göran -

I'm not sure what the UI will look like but in general I like what 
you're proposing. The most important property here is predictability. As 
long as people know the constraints they can work within them, so 
requiring that for a doIt to be reversible it needs to have an 
"anti-doIt" is just fine (and was my preferred choice after reading the 
first paragraph of your message ;-) For the records, many of the 
problems I've seen with Monticello come from it doing things seemingly 
unpredictably and consequently you can't address them beforehand since 
you don't really know what might go wrong. It's a really important 
lesson to keep in mind.

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.

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.

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 :-)

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.

Lastly, class initialization. Same rules apply as above, just be 
predictable. 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.

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 ;-)

Cheers,
   - Andreas


Göran Krampe wrote:
> Hi!
> 
> Ok, one of the things we did in Brest was to add first support for DoIts 
> in Deltas. Up to now a recording DSDelta creates DSChange instances 
> (subclasses thereof) for each kind of change.
> 
> This means we DO NOT create doits in Deltas for method removals, class 
> creation nor class initialization (although that actually will cause 
> code to be run that of course potentially could screw things up) etc. 
> All these three are instead covered by different DSChange objects.
> 
> BUT... if we want to replace Changesets we need to support doits at 
> least as much as Changesets do. A Changeset can have a preamble and a 
> postscript with code in them. This means you can include an arbitrary 
> snippet of code to be executed pre/post the install of the Changeset.
> 
> A Delta consists of an sequence of DSChanges. This means that we can put 
> a DSDoIt anywhere in that list - even in the middle. Ok, so we are 
> "better" than Changesets :)
> 
> BUT... a Delta is meant to be reversable etc, so by allowing doIts we 
> are moving into undefined territory. Thus, the medicine looks like this:
> 
> 1. We do NOT record doits by default. Changesets actually don't record 
> doits either btw. :) But you can tell a DSDelta to record doits which of 
> course will log tons of crap, but it can be interesting to log and then 
> just prune it away. And yes, we could analyze the actual doits and do 
> some auto-pruning but whatever...
> 
> 2. If you record a DSDoIt OR add one manually it will NOT be executed 
> when the Delta is applied! You need to EXPLICITLY mark it using 
> executeOnApply: true.
> 
> 3. If a Delta contains a DSDoIt marked to execute on apply, the Delta 
> will be considered NON-reversable UNLESS you also supply an antiDoIt 
> code snippet! In this case DSDelta>>isReversable would say "true" BUT 
> this only means that it COULD PERHAPS be reversed.
> 
> Finally we will explicitly explain to the developer that:
> --------------
> If you still have a DoIt in your Delta that you intend to distribute, 
> make sure that:
>         - It can cope with missing globals/classes!
>         - It can be safely run multiple times without problems!
>         - You marked it to be run on apply using #executeOnApply:. This 
> is an EXPLICIT step for a reason.
>         - If it should do able to be REVERSED you need to supply an 
> antiDoIt.
>         - Put DoIts preferrably first or last in the Delta to avoid 
> strange situations. Remember that even if changes are ordered they may 
> be applied atomically by an applier.
> 
> NOTE: A Delta containing any DoIt will be visualized as "dangerous" in 
> the user interfaces so please avoid them unless you REALLY need them.
> -------------
> 
> 
> Well, something like that. Does the above handling of DoIts seem 
> reasonable? Any other suggestions or enhancements to all this?
> 
> Sidenote: If a Delta is set to record doits it will end up with *both* a 
> class creation change and a class creation doit. As the code stands 
> today the Delta can't tell the difference. I was inclined to perhaps use 
> the #kind: selector in AbstractEvent to distinguish the doits produced 
> by our own tools from doits performed by the developer himself in 
> ParagraphEditor etc. But I dropped that for now. As described above such 
> a recorded DoIt will not be executed on apply anyway - to be recording 
> doits is mainly useful as a "true log" of everything happening.
> 
> regards, Göran
> 
> PS. Any interesting thoughts about class initalization is also 
> appreciated. A Changeset slaps in an initialize-doit when being filedout 
> if touched class definitions (I think) has such a selector. Should we do 
> something similar on apply?
> 
> 
> 




More information about the Squeak-dev mailing list