Moodle
  1. Moodle
  2. MDL-30604

Add Moodle 2.2.0 upgrade line to all the upgrade.php scripts

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.3
    • Fix Version/s: 2.2.1
    • Component/s: Installation
    • Labels:
    • Testing Instructions:
      Hide

      1) find . -name upgrade.php | xargs grep -B2 'return true;' check all upgrade scripts have at least one upgrade line. Exception! the change to enrol/guest/db/upgrade.php has the comments before 2011112901 that has been created this weel.
      2) check that the 3 commands in the description of the issue return 0 files.

      Show
      1) find . -name upgrade.php | xargs grep -B2 'return true;' check all upgrade scripts have at least one upgrade line. Exception! the change to enrol/guest/db/upgrade.php has the comments before 2011112901 that has been created this weel. 2) check that the 3 commands in the description of the issue return 0 files.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-30604_m23
    • Rank:
      33405

      Description

      In order to have it properly detected for the future it would be great to add to all the upgrade.php scripts some lines like these:

      // Moodle v2.2.0 release upgrade line
      // Put any upgrade step following this
      

      exactly before the "return true;" present in all the scripts.

      I think it's ok to do that both in the 22_STABLE and master branches, so they will allow quickly find where 2.2.0 started and act once we decide future requirements.

      Command to detect all the upgrade.php files not having those lines:

      find . -name upgrade.php | xargs grep -l 'function.*xmldb_.*_upgrade' | grep '/db/' | xargs grep -L 'Moodle v2.2.0 release upgrade'
      

      Commands to detect that we have not added the lines to incorrect files:

      grep -lr 'Moodle v2.2.0 release upgrade' * | grep -v '/db/'
      grep -lr 'Moodle v2.2.0 release upgrade' * | xargs grep -L 'function.*xmldb_.*_upgrade'
      

      Ciao

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          Apu, I've left this assigned to you because you did it for the 21_STABLE branch. Feel free to re-assign / raise it to MdR if necessary. TIA!

          Show
          Eloy Lafuente (stronk7) added a comment - Apu, I've left this assigned to you because you did it for the 21_STABLE branch. Feel free to re-assign / raise it to MdR if necessary. TIA!
          Hide
          Aparup Banerjee added a comment -

          aha, thanks Eloy, bringing this into the stable sprint.

          Show
          Aparup Banerjee added a comment - aha, thanks Eloy, bringing this into the stable sprint.
          Hide
          Aparup Banerjee added a comment -

          up for peer review now

          Show
          Aparup Banerjee added a comment - up for peer review now
          Hide
          Rossiani Wijaya added a comment -

          This looks fine.

          Thanks Apu.

          Show
          Rossiani Wijaya added a comment - This looks fine. Thanks Apu.
          Hide
          Rossiani Wijaya added a comment -

          Submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Submitting for integration review.
          Hide
          Petr Škoda added a comment -

          Hi, this is probably not necessary for master branch because MDL-30610 means there will not be any 2.2 upgrade code there...

          Show
          Petr Škoda added a comment - Hi, this is probably not necessary for master branch because MDL-30610 means there will not be any 2.2 upgrade code there...
          Hide
          Aparup Banerjee added a comment -

          ok understood but its probably necessary for separating the code between 2.2 - 2.2.1 - .. - 2.3 for clean up later ?

          anyway, Atm the idea is to keep 2.2 and master same for now, not sure how this applies to comments , I'll leave it to the integrators.

          Show
          Aparup Banerjee added a comment - ok understood but its probably necessary for separating the code between 2.2 - 2.2.1 - .. - 2.3 for clean up later ? anyway, Atm the idea is to keep 2.2 and master same for now, not sure how this applies to comments , I'll leave it to the integrators.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Apu,

          I'm not sure how you have detected the scripts needing that but, hehe, it seems that there are some dozens missing.

          I'll keep this under integration in the mean time... ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Apu, I'm not sure how you have detected the scripts needing that but, hehe, it seems that there are some dozens missing. I'll keep this under integration in the mean time... ciao
          Hide
          Aparup Banerjee added a comment -

          aha, some thing really weird happened , all fixed now.
          also it looks like ./enrol/guest/db/upgrade.php was just integrated but since its post 2.2 it probably didn't need the upgrade line.

          Show
          Aparup Banerjee added a comment - aha, some thing really weird happened , all fixed now. also it looks like ./enrol/guest/db/upgrade.php was just integrated but since its post 2.2 it probably didn't need the upgrade line.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I've integrated this adding 2 missing commits (finally I added it to the enrol/guest one) and deleting from an incorrect one (question/upgrade).

          At the same time, I've updated the commands to detect candidates and also some to detect incorrect ones (better for future clones of this).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I've integrated this adding 2 missing commits (finally I added it to the enrol/guest one) and deleting from an incorrect one (question/upgrade). At the same time, I've updated the commands to detect candidates and also some to detect incorrect ones (better for future clones of this). Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Passing, as far as I already reviewed all the uses on integration. And the scripts in the issue description also say it's good.

          Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Passing, as far as I already reviewed all the uses on integration. And the scripts in the issue description also say it's good. Thanks!
          Hide
          Aparup Banerjee added a comment -

          Thanks Eloy, i actually read thru the question/upgrade one, theres no DB manager etc there but it did seem to be about updating stuff ('is what this update does'), anyway just 2c there. aha, just read that its only for */db/ files.. The End.

          Show
          Aparup Banerjee added a comment - Thanks Eloy, i actually read thru the question/upgrade one, theres no DB manager etc there but it did seem to be about updating stuff ('is what this update does'), anyway just 2c there. aha, just read that its only for */db/ files.. The End.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yes, you did it!

          Now your code is part of the best weeklies released ever, many thanks!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yes, you did it! Now your code is part of the best weeklies released ever, many thanks! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: