Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Database SQL/XMLDB
    • Labels:

      Description

      Once the rest of the META is closed, it may be a good timing to clean some coding minor issues under ddl, dml and xmldb editor:

      • some phpdocs.
      • 3-slash comments.
      • ...
      • ...

      Only for 2.3, so we can start with a nicer codebase there. Ciao

        Gliffy Diagrams

          Activity

          Hide
          Petr Skoda added a comment -

          yay, I am really looking forward to this...

          Show
          Petr Skoda added a comment - yay, I am really looking forward to this...
          Hide
          Petr Skoda added a comment -

          I am going to fix old PHP4-isms at the same time - that is constructors, =&, etc.

          Show
          Petr Skoda added a comment - I am going to fix old PHP4-isms at the same time - that is constructors, =&, etc.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Lol, reviewing these:

          protected=$( grep -r ' *protected \$[a-z_]*' lib/xmldb/* | \
                       sed -e 's/.*$\([a-z_]*\);/\1/g' | xargs | \
                       sed -e 's/ /|/g' ) && \
          grep -rP "[^s]\->(${protected})[^a-z_=-]" admin/tool/xmldb/* | \
          grep -v 'dbdir\->path'
          

          aka: review that all protected xmldb_xxxx attributes aren't accessed publicly.

          Integrating soon...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Lol, reviewing these: protected=$( grep -r ' *protected \$[a-z_]*' lib/xmldb/* | \ sed -e 's/.*$\([a-z_]*\);/\1/g' | xargs | \ sed -e 's/ /|/g' ) && \ grep -rP "[^s]\->(${protected})[^a-z_=-]" admin/tool/xmldb/* | \ grep -v 'dbdir\->path' aka: review that all protected xmldb_xxxx attributes aren't accessed publicly. Integrating soon...ciao
          Hide
          Petr Skoda added a comment -

          yeah, I clicked through all pages of the XMLDB editor to make sure it is not used, phpunit tests shoudl verify the lib/xmldb/ access in upgrades and install

          Show
          Petr Skoda added a comment - yeah, I clicked through all pages of the XMLDB editor to make sure it is not used, phpunit tests shoudl verify the lib/xmldb/ access in upgrades and install
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Finally there were only one protected access violated. I've fixed it with extra commit, using actions errors instead of structure ones.

          So integrated. Thanks and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Finally there were only one protected access violated. I've fixed it with extra commit, using actions errors instead of structure ones. So integrated. Thanks and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Passing:

          • All 4DB ran all the tests ok (just the ususal 2 exceptions in mssql and the 1 fail in oracle).
          • Tested all the options of the editor (unless I've left some "button" without try). All worked as expected.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Passing: All 4DB ran all the tests ok (just the ususal 2 exceptions in mssql and the 1 fail in oracle). Tested all the options of the editor (unless I've left some "button" without try). All worked as expected. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          And this has been incorporated to all the weekly builds and also, to Moodle 2.3 Release Candidate 1, yay!

          Many, many thanks for your hard work!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - And this has been incorporated to all the weekly builds and also, to Moodle 2.3 Release Candidate 1, yay! Many, many thanks for your hard work! Ciao

            People

            • Assignee:
              Petr Skoda
              Reporter:
              Eloy Lafuente (stronk7)
              Integrator:
              Eloy Lafuente (stronk7)
              Tester:
              Eloy Lafuente (stronk7)
              Participants:
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: