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:
    • Rank:
      38674

      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

        Activity

        Hide
        Petr Škoda added a comment -

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

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

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

        Show
        Petr Škoda 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 Škoda 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 Škoda 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 Škoda
            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: