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

          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