[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.
cheers,
r
More information about the asdf-devel
mailing list