[asdf-devel] Pushed proposed fix for Xach's bug

Robert Goldman rpgoldman at sift.info
Tue Sep 6 01:10:44 UTC 2011

On 9/4/11 Sep 4 -10:18 AM, Faré wrote:
>> OK, sorry about that.  I did the bin/bump-version, and pushed.
> No problem. Thanks. Just be sure to bump-version before you push to master.
>> As for pushing to master, while I am not perfectly confident in the
>> patch, it passes all the tests, and I'm as confident as I can get
>> without people on the bleeding edge giving it a test.
> I looked at the code, it looks great.
> I like how you implement those reinitialize-instance methods.
>> I
>> believe only the SYSTEM objects will be reused, since once the children
>> slot of the SYSTEM is cleared, the previous child COMPONENTs should
>> become unreachable.
> Maybe we could have made the whole thing simpler by only implementing
> reinitialize-instance for SYSTEM objects?
> But your approach is cleaner, in a way, so I wouldn't revise that.

This seems like a matter of taste, and I confess that I see arguments
for both sides:

1.  Only implement methods for SYSTEM objects:  makes it clear that
these are the only types of object we expect to see reused.

2.  Implement methods for COMPONENT, MODULE, and SYSTEM, each handling
its own slots:  the advantage here is that the code for reinitialization
is syntactically close to the code defining the slots that are
reinitialized.  That made the methods easier for me while I was writing
them, and I would hope that this would make it easier for anyone
maintaining the code to realize that they need to maintain these methods.

Obviously, I chose the latter, but I see arguments both ways.

>> I understand you are busy with other things, but if you get a chance,
>> LMK what you think.
> Looks great to me.
> BTW, if you have more time than I for ASDF, you might give a look to
> Juanjo's latest patch for ECL...

I'm not an ECL user myself, so I'm not sure how to test or evaluate
this.  I'll try to take a look and think about it.


More information about the asdf-devel mailing list