Moodle
  1. Moodle
  2. MDL-30300

Improve question behaviour API: required_question_definition_type -> is_compatible_question

    Details

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

      Description

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

        Issue Links

          Activity

          Hide
          Oleg Sychev added a comment -

          Getting accustomed to the new workflow....

          Show
          Oleg Sychev added a comment - Getting accustomed to the new workflow....
          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          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
          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
          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
          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
          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
          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
          Eloy Lafuente (stronk7) added a comment -

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

          Thanks! Closing...

          Show
          Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Thanks! Closing...
          Hide
          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
          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
          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
          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
          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
          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
          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
          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: