Removes MODULARIZE_INSTANCE option #11037
Conversation
Removes all references to MODULARIZE_INSTANCE, as well as the tests and sub-test runs that covered the behavior. Simplified the logic in some places that had to check for MODULARIZE but not MODULARIZE_INSTANCE
sbc100
left a comment
sbc100
left a comment
There was a problem hiding this comment.
Oh yeah! I'm loving all the code deletion thats getting done these days.
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
Thanks @lourd !
Please add a mention to the changelog as well.
I'd suggest adding to the overview here, and to the changelog, that we may reintroduce something like MODULARIZE_INSTANCE eventually, if there is demand, and after we finish the major reworking of MODULARIZE.
In particular, originally MODULARIZE_INSTANCE was literally just a few lines of code to add an instance creation, but clearly after reading the removed code here it ended up quite complex! Maybe we can figure out a way to do it in a simple way in the future (again, only if there's demand, and after finishing the new MODULARIZE).
|
We should make some effort to see if there are still use cases for this. While it seems like it should be easy for people to live without this, maybe we're missing something. We should at least wait a few days before landing it, and should post to the mailing list as well. (I can do that if you don't want to.) |
|
Totally, sounds good. If you could please post to the mailing list about it that'd be great, I've not done that before. |
|
Ok, I posted on the mailing list, and no responses in 5 days. No concerns here either, so let's land it! |
3 participants
As discussed in #10697, we're removing the
MODULARIZE_INSTANCEbuild option for these reasons:MODULARIZEoptionMODULARIZEhandling in a non-trivial way, making development and maintenance more difficult.If you use this build option, please let us know! We may re-add it in the future after reworking the
MODULARIZEoption, work which has been started in #10697.