Details

    • Type: Sub-task
    • Status: Closed
    • Priority: 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

          Attachments

            Activity

            Hide
            skodak Petr Skoda added a comment -

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

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

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

            Show
            skodak Petr Skoda added a comment - I am going to fix old PHP4-isms at the same time - that is constructors, =&, etc.
            Hide
            stronk7 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
            stronk7 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
            skodak 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
            skodak 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
            stronk7 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
            stronk7 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
            stronk7 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
            stronk7 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
            stronk7 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
            stronk7 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:
                skodak Petr Skoda
                Reporter:
                stronk7 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:
                  Fix Release Date:
                  25/Jun/12