Details

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

      Gliffy Diagrams

        Activity

        Hide
        nebgor 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
        nebgor 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
        mudrd8mz David Mudrak added a comment -

        This has been just fixed in my branch.

        Show
        mudrd8mz David Mudrak added a comment - This has been just fixed in my branch.
        Hide
        andyjdavis 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
        andyjdavis 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
        nebgor 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
        nebgor 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
        mudrd8mz 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
        mudrd8mz 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
        mudrd8mz David Mudrak added a comment -

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

        Show
        mudrd8mz 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: