Passes all of the unit tests (including the new one based on Xach's test case).
Please, all of you, have a peek at the diffs (possibly I should have squashed the changeset to make this easier, sorry), and the commit message, and test it as soon as possible.
I have not bumped the version number, as this is still a work in progress subject to review.
best, r
On Fri, Sep 2, 2011 at 18:29, Robert Goldman rpgoldman@sift.info wrote:
Passes all of the unit tests (including the new one based on Xach's test case).
Please, all of you, have a peek at the diffs (possibly I should have squashed the changeset to make this easier, sorry), and the commit message, and test it as soon as possible.
I have not bumped the version number, as this is still a work in progress subject to review.
If you push a code change to master, you MUST bump the version number. If you don't feel confident about the code, please don't push to master.
Didn't you say it would be better to throw the old object and create a new one than to try to reinitialize those component instances?
—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org
On 9/2/11 Sep 2 -5:41 PM, Faré wrote:
On Fri, Sep 2, 2011 at 18:29, Robert Goldman rpgoldman@sift.info wrote:
Passes all of the unit tests (including the new one based on Xach's test case).
Please, all of you, have a peek at the diffs (possibly I should have squashed the changeset to make this easier, sorry), and the commit message, and test it as soon as possible.
I have not bumped the version number, as this is still a work in progress subject to review.
If you push a code change to master, you MUST bump the version number. If you don't feel confident about the code, please don't push to master.
OK, sorry about that. I did the bin/bump-version, and pushed.
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.
Didn't you say it would be better to throw the old object and create a new one than to try to reinitialize those component instances?
Yes, but Christophe convinced me that this was The Wrong Thing, because methods defined on (EQL (FIND-SYSTEM <foo>)) would be broken by doing so. That led me to the REINITIALIZE-INSTANCE approach. Note, though, that it should have the same semantics --- the COMPONENTs are cleared, and that means that the children of the system will be made new. So 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.
I understand you are busy with other things, but if you get a chance, LMK what you think.
Best, R
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.
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...
—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org
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
-
Faré -
Robert Goldman