Code review comment for lp:~divmod-dev/divmod.org/829869-explicit-type-dropping

Tristan Seligmann (mithrandi) wrote :

I wrote a more detailed explanation of this branch on IRC, which really belongs here, so here you go:

<idnar> lvh: so basically, the situation is
<idnar> lvh: that branch deletes two tests that test some functionality
<idnar> lvh: the only code that makes use of that functionality in any way is those two tests; there isn't even anything else in the test suite that uses it, nevermind any application code that I'm aware of
<idnar> lvh: furthermore, making use of that functionality in some application in a useful fashion is extremely difficult, to the point where I would conjecture nobody has done so
<idnar> lvh: so that's why the branch just deletes the tests
<exarkun> the reason I am cautious, of course, is "any application code that I'm aware of"
<idnar> the functionality is: being able to define an item type with a certain type name, break all references to that item type, then define a different
<idnar> item with the same typeName
<idnar> (well, or the same item)
<exarkun> obscure functionality, to be sure
<idnar> perhaps I'm exaggerating the difficulty of using this functionality usefully
<idnar> well, I guess I should make a correction there
<idnar> "break all references" is not the thing you need to do; "ensure the Item subclass is deallocated" is the thing you need to do
<idnar> the unit test tries to accomplish this by breaking all of the references it has to the subclass, which works on CPython but not on PyPy
<lvh> idnar: Okay, so it's not less obscure than I imagined it. I can't imagine any application code either.
<lvh> (Again, that doesn't cover exarkun's valid comment about what we can imagine being irrelevant)
<idnar> I guess one alternative would be to skip the tests, but only on pypy
<exarkun> but we could always re-introduce it as a feature with an explicit API, later. I don't think anyone not in this channel has much right to complain about super obscure gc interaction behaviors in axiom changing.
<exarkun> (something like store.unregisterType(...), store.registerType(...))

« Back to merge proposal