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 _______________________________________________ asdf-devel mailing list [email protected] http://lists.common-lisp.net/cgi-bin/mailman/listinfo/asdf-devel
