Hi Tim, should I do it? Is it really not used any more?

The situation is this:

1. The questions table belongs to Moodle core.

2. tables like question_multichoice belong to qtype_mutichoice (really, one day, we should rename those tables to be frankenstyle).

3. In the upgrade from 1.9 -> 2.0, when we added the missing ...format columns, often to upgrade some question_multichoice columns, you need to know what question.questiontypeformat was.

4. But the core upgrade 1.9 -> 2.0 sometimes changed question.questiontypeformat.

5. Core upgrade runs before qtype upgrade.

So, anyway, that is why we had to make a copy of question.questiontypeformat as oldquestiontypeformat in lib/db/upgrade.php, before we started changing it. But, it was only needed during that upgrade.

Now that we are past 2.3, we can drop the column. I am happy for you to do it. If no, I will.

Hehe, I do not understand your explanation completely, but it is great we can safely drop the column in 2.3x and master upgrade code.

Next time please always make sure that the install.xml files match the current database structures, otherwise we have huge problems in scripts like dbtransfer or any scheme validation tool if we ever decide to write it.

I am going to write the upgrade code myself because I will need to coordinate this with other install.xml cleanup, thanks!

sending for pear review, thanks

+1 from me.

Ut oh, this has only had pear review, should I be concerned? :-P

But seriously, i'm not sure about dropping a field like this in the middle of stable branch. I know it shouldn't have any implications, just would like some integrator +1's for that.

Hi.

This gets a +0.5 from me.
Given that this field is not used in core, was created for the requirement of upgrade code, and that the associated upgrade code has now been removed I think its removal from the 23 upgrade branch is tolerable.
There is always that slim chance someone is relying on that field being in the database, or present in records returned from the database. However I think in this situation the chance that someone was relying on it is remote enough to void this concern.

Cheers
Sam

The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P
Apologies for the inconvenience, this will be integrated next week. Thanks for your collaboration & ciao

+0.5 here too.

I just would add one more testing step about restoring one 1.9.x course with questions. I'm 99% sure it's completely unrelated but, as far as it mimics some of the old upgrade steps, better check it works without any warn/error depending of that DB field.

Ciao

Thanks Eloy, I really must get into my reviewing mind backup/restore implications, i'm not very good at remembering it.

We have over 1.0 integrator vote now, so can integrate.

Integrated, thanks

This looks good.

Test passed.

Fixed STOP Closed STOP Thanks STOP

Yay, imagination! Ciao