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

Codechecker issues on MDL-39810 commit

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: 2.6
    • Fix Version/s: 2.5.4, 2.6.1
    • Component/s: Libraries
    • Labels:

      Description

      Reading about code checker in https://moodle.org/mod/forum/discuss.php?d=244921#p1062134 I realized that I made an error when submitting the MDL-39810 PR for 2.6: I've used $compat_view instead of $compatview.

      The 2.5 branch, though affected by a similar issue, should not be modified because of $supports_json_contenttype is more readable than $supportsjsoncontenttype and, at that time, I tried to mimic the new function introduced in 2.6+, core_useragent::supports_json_contenttype().

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            matteo Matteo Scaramuccia added a comment -

            Hi Eloy,
            could peer review my PR? I'd like not to be remembered as one of those that have broken the naming conventions even if any "recent code" should be following the style since a couple of years

            TIA,
            Matteo

            Show
            matteo Matteo Scaramuccia added a comment - Hi Eloy, could peer review my PR? I'd like not to be remembered as one of those that have broken the naming conventions even if any "recent code" should be following the style since a couple of years TIA, Matteo
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            LOL, LOL!

            will do tomorrow, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - LOL, LOL! will do tomorrow, thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Thanks Matteo,

            it looks ok. I just would say: apply for it only in master (or apply for it in all the branches where it was originally implemented).

            The first option is the default one and the 2nd is acceptable in this case because the original issue is really recent.

            I'll leave that in integrator's hands... sending to integration. Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Thanks Matteo, it looks ok. I just would say: apply for it only in master (or apply for it in all the branches where it was originally implemented). The first option is the default one and the 2nd is acceptable in this case because the original issue is really recent. I'll leave that in integrator's hands... sending to integration. Ciao
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            matteo Matteo Scaramuccia added a comment -

            Rebased, pending the integrators' decision about including the 2.5 branch too.

            Show
            matteo Matteo Scaramuccia added a comment - Rebased, pending the integrators' decision about including the 2.5 branch too.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Matteo - this has been integrated now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Matteo - this has been integrated now.
            Hide
            dobedobedoh Andrew Nicols added a comment -

            Unit tests run on master - passed
            Installing phpunit onto 26 and running regression tests on 25...

            Show
            dobedobedoh Andrew Nicols added a comment - Unit tests run on master - passed Installing phpunit onto 26 and running regression tests on 25...
            Hide
            dobedobedoh Andrew Nicols added a comment -

            26 unit tests pass
            Regression tests pass

            Show
            dobedobedoh Andrew Nicols added a comment - 26 unit tests pass Regression tests pass
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks for the code, its now upstream!

            Heres a fun trick to try in the spirit of Friday the 13th.
            I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks for the code, its now upstream! Heres a fun trick to try in the spirit of Friday the 13th. I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/Jan/14