Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-30300

Improve question behaviour API: required_question_definition_type -> is_compatible_question

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.2, 2.2
    • Fix Version/s: 2.1.3
    • Component/s: Questions
    • Labels:
      None

      Description

      See http://moodle.org/mod/forum/discuss.php?d=190395. The proposed new API is simpler and more flexible.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            oa_sychev Oleg Sychev added a comment -

            Getting accustomed to the new workflow....

            Show
            oa_sychev Oleg Sychev added a comment - Getting accustomed to the new workflow....
            Hide
            oa_sychev Oleg Sychev added a comment -

            Good thing, I coudn't find nothing wrong there.

            Now new behaviours are free to query question compatibility whenever way they need, which is much more flexible.

            Show
            oa_sychev Oleg Sychev added a comment - Good thing, I coudn't find nothing wrong there. Now new behaviours are free to query question compatibility whenever way they need, which is much more flexible.
            Hide
            oa_sychev Oleg Sychev added a comment -

            Well, I don't seem to have any other workflow commands than start and finish peer review. It's probably you who could send this for integration...

            Show
            oa_sychev Oleg Sychev added a comment - Well, I don't seem to have any other workflow commands than start and finish peer review. It's probably you who could send this for integration...
            Hide
            timhunt Tim Hunt added a comment -

            Thanks Oleg. That is correct. All you can do is add a comment with any review comments, and whether you think it is ready for integration.

            I'm submitting for integration now.

            Show
            timhunt Tim Hunt added a comment - Thanks Oleg. That is correct. All you can do is add a comment with any review comments, and whether you think it is ready for integration. I'm submitting for integration now.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Simple Q: So base question_behaviour->is_compatible_question() will become abstract at a given future release? Would it be interesting to add some @todo about that? Or is it intended to continue supporting old 3rd-part behaviors forever?

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Simple Q: So base question_behaviour->is_compatible_question() will become abstract at a given future release? Would it be interesting to add some @todo about that? Or is it intended to continue supporting old 3rd-part behaviors forever?
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
            Hide
            nebgor Aparup Banerjee added a comment -

            unit tests ran fine for me.
            questions behaviour don't seem to have changed. I went through a variety in a quiz. Passing this very generally as per generalized test description.

            Show
            nebgor Aparup Banerjee added a comment - unit tests ran fine for me. questions behaviour don't seem to have changed. I went through a variety in a quiz. Passing this very generally as per generalized test description.
            Hide
            timhunt Tim Hunt added a comment -

            I did not feel the need to be precise about when we would stop supporting the old way. Actually, behaviours are new, and I only know of three contrib ones. (One by me, one my Joseph Reseau, and one by Oleg) so I will probably wait until I know those are fixed and then take out the old code. I'll make a MDL for it, rather than polluting the code with TODOs: MDL-30320

            Show
            timhunt Tim Hunt added a comment - I did not feel the need to be precise about when we would stop supporting the old way. Actually, behaviours are new, and I only know of three contrib ones. (One by me, one my Joseph Reseau, and one by Oleg) so I will probably wait until I know those are fixed and then take out the old code. I'll make a MDL for it, rather than polluting the code with TODOs: MDL-30320
            Hide
            timhunt Tim Hunt added a comment -

            Aparup, I wrote vague (& short!) test instructions because I know that the unit tests for question behaviours are very good.

            Show
            timhunt Tim Hunt added a comment - Aparup, I wrote vague (& short!) test instructions because I know that the unit tests for question behaviours are very good.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            U P S T R E A M I Z E D !

            Thanks! Closing...

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Thanks! Closing...
            Hide
            nebgor Aparup Banerjee added a comment -

            Tim, thats cool then! I'm now increasing my trust on unit tests by 1%. :-p

            Just noting that we should try to utilize the testing time & testing instructions to actually utilize human brain testing time.
            Relying on Unit tests alone is a potential blind spot for regressions, we need a human to test the fixes (and new unit tests too if any.)

            Show
            nebgor Aparup Banerjee added a comment - Tim, thats cool then! I'm now increasing my trust on unit tests by 1%. :-p Just noting that we should try to utilize the testing time & testing instructions to actually utilize human brain testing time. Relying on Unit tests alone is a potential blind spot for regressions, we need a human to test the fixes (and new unit tests too if any.)
            Hide
            timhunt Tim Hunt added a comment -

            Aparup, go and look at some of the other testing instructions I have written. (E.g. MDL-30182, MDL-30122, MDL-29808, MDL-29627, MDL-29570). I am quite capable of writing detailed instructions when necessary. In this case, the nature of the change meant that it was likely to either break completely, or just work. Writing longer testing instructions would just have been a waste of everyone's time. Did you look at that patch, and see what had changed, before criticising the appropriateness of the testing instructions?

            Show
            timhunt Tim Hunt added a comment - Aparup, go and look at some of the other testing instructions I have written. (E.g. MDL-30182 , MDL-30122 , MDL-29808 , MDL-29627 , MDL-29570 ). I am quite capable of writing detailed instructions when necessary. In this case, the nature of the change meant that it was likely to either break completely, or just work. Writing longer testing instructions would just have been a waste of everyone's time. Did you look at that patch, and see what had changed, before criticising the appropriateness of the testing instructions?
            Hide
            nebgor Aparup Banerjee added a comment -

            Tim,
            I've appreciated your verbose testing instructions many times when doing testing. I'm not questioning you or your ability here, i was merely noting that depending only on unit tests could be a blind spot during testing. Note that i was tester on this , not integrator.

            All i'm trying to say is adding something to the effect of : "the unit tests for question behaviours are very good , (its) likely to either break completely, or just work" to the instructions would help communicate to any tester the full scope and depth of testing necessary when we stick to general instructions. Its better that we don't depend on testers judging the patch too (tester bias etc).

            hope you've understood me (not necessarily agree), thats all.

            Show
            nebgor Aparup Banerjee added a comment - Tim, I've appreciated your verbose testing instructions many times when doing testing. I'm not questioning you or your ability here, i was merely noting that depending only on unit tests could be a blind spot during testing. Note that i was tester on this , not integrator. All i'm trying to say is adding something to the effect of : "the unit tests for question behaviours are very good , (its) likely to either break completely, or just work" to the instructions would help communicate to any tester the full scope and depth of testing necessary when we stick to general instructions. Its better that we don't depend on testers judging the patch too (tester bias etc). hope you've understood me (not necessarily agree), thats all.
            Hide
            timhunt Tim Hunt added a comment -

            Yes. I get your point. Thank you for challenging me on this. I may not like it when it happens, but it is important to think about what we are doing from time to time.

            Show
            timhunt Tim Hunt added a comment - Yes. I get your point. Thank you for challenging me on this. I may not like it when it happens, but it is important to think about what we are doing from time to time.

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  28/Nov/11