Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: DEV backlog
    • Fix Version/s: None
    • Component/s: Course
    • Labels:
      None
    • Rank:
      1230

      Activity

      Hide
      Aparup Banerjee added a comment -

      i've got the code up review for label @ https://github.com/nebgor/moodle/compare/mistress...MDL-27445

      note:
      upon running backup/converter/moodle1 test with the attached 1.9 label backup file, i'm still getting the error below :

      Exception: backup/converter/moodle1/simpletest/testlib.php / ► moodle1_converter_test / ► test_convert_run_convert
      Unexpected PHP error [Undefined property: moodle1_parser_processor::$courseid] severity [E_NOTICE] in [/home/aparup/mcode/m20/mysql/moodle/backup/converter/moodle1/lib.php line 490]

      but i think this can be expected while running unit test without course maybe.

      Show
      Aparup Banerjee added a comment - i've got the code up review for label @ https://github.com/nebgor/moodle/compare/mistress...MDL-27445 note: upon running backup/converter/moodle1 test with the attached 1.9 label backup file, i'm still getting the error below : Exception: backup/converter/moodle1/simpletest/testlib.php / ► moodle1_converter_test / ► test_convert_run_convert Unexpected PHP error [Undefined property: moodle1_parser_processor::$courseid] severity [E_NOTICE] in [/home/aparup/mcode/m20/mysql/moodle/backup/converter/moodle1/lib.php line 490] but i think this can be expected while running unit test without course maybe.
      Hide
      David Mudrak added a comment -

      This has been just fixed in my branch.

      Show
      David Mudrak added a comment - This has been just fixed in my branch.
      Hide
      Andrew Davis added a comment -

      Commencing peer review in 3... 2... 1...

      I'd recommend either listing a single commit as your diff URL (https://github.com/nebgor/moodle/commit/49250e8b4d7491e9cacf3cc7edf3dc3a0f8bf3d5) or pushing the backup-convert branch to your github repo giving you a diff URL like https://github.com/andyjdavis/moodle/compare/backup-convert...MDL-27448_resource_restore. Either way would make it easier to see whats going on.

      I think you've got the wrong MDL number in your commit message. The only commit I can see in the diff by you references MDL-27439 in the commit message. MDL-27439 is to do with the assignment module although the code in that commit is do with label handling (ie this issue). Just a copy/paste slip up I assume. Possibly worth fixing. dont know.

      Otherwise it looks fine

      Show
      Andrew Davis added a comment - Commencing peer review in 3... 2... 1... I'd recommend either listing a single commit as your diff URL ( https://github.com/nebgor/moodle/commit/49250e8b4d7491e9cacf3cc7edf3dc3a0f8bf3d5 ) or pushing the backup-convert branch to your github repo giving you a diff URL like https://github.com/andyjdavis/moodle/compare/backup-convert...MDL-27448_resource_restore . Either way would make it easier to see whats going on. I think you've got the wrong MDL number in your commit message. The only commit I can see in the diff by you references MDL-27439 in the commit message. MDL-27439 is to do with the assignment module although the code in that commit is do with label handling (ie this issue). Just a copy/paste slip up I assume. Possibly worth fixing. dont know. Otherwise it looks fine
      Hide
      Aparup Banerjee added a comment -

      thanks!
      edited the message.
      diff updated to : https://github.com/nebgor/moodle/commit/49bef1eb52ac6c4efb16d888ff4b1ba1ffb90c08

      (Hm, the diff against david's branch https://github.com/nebgor/moodle/compare/backup-convert...MDL-27445
      could end up broken if david updates his branch and i didn't. i wonder if i could've diffed against his branch in his repo.)

      Show
      Aparup Banerjee added a comment - thanks! edited the message. diff updated to : https://github.com/nebgor/moodle/commit/49bef1eb52ac6c4efb16d888ff4b1ba1ffb90c08 (Hm, the diff against david's branch https://github.com/nebgor/moodle/compare/backup-convert...MDL-27445 could end up broken if david updates his branch and i didn't. i wonder if i could've diffed against his branch in his repo.)
      Hide
      David Mudrak added a comment -

      Andrew is right - to integrate your branch into my one, I need to know the name of your branch with the subtask implemented. And if you provide diff URL that shows just your changes, it makes my life easier.

      Please note that the fact that your copy of my branch is slightly outdated is not problem in github. I always review the change before the actual merge anyway.

      In this case, the single objection is against the obsolete get_moduleid() call and I fix that via additional commit.

      Show
      David Mudrak added a comment - Andrew is right - to integrate your branch into my one, I need to know the name of your branch with the subtask implemented. And if you provide diff URL that shows just your changes, it makes my life easier. Please note that the fact that your copy of my branch is slightly outdated is not problem in github. I always review the change before the actual merge anyway. In this case, the single objection is against the obsolete get_moduleid() call and I fix that via additional commit.
      Hide
      David Mudrak added a comment -

      Label support now included in my pre-integration branch. Thanks!

      Show
      David Mudrak added a comment - Label support now included in my pre-integration branch. Thanks!

        People

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

          Dates

          • Created:
            Updated:
            Resolved: