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