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

textlib functions should be called as static methods

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.1
    • Fix Version/s: 2.3
    • Component/s: Other
    • Labels:
    • Testing Instructions:
      Hide

      no idea how to test it, I did the patch using trivial search replace, there should not be any regressions...

      • Running and passing all the tests under master may be a good enough way to control that nothing has gone out of control
      Show
      no idea how to test it, I did the patch using trivial search replace, there should not be any regressions... Running and passing all the tests under master may be a good enough way to control that nothing has gone out of control
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w10_MDL-31301_m23_textlibcleanup

      Description

      In MDL-29027 textlib functions became static, but calling for this function is not changed. They are still called as textlib->substr(). It should be changed to textlib::substr().

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              rajeshtaneja Rajesh Taneja added a comment -

              Some string related functions in lib/moodlelib.php can be moved to textlib.

              Show
              rajeshtaneja Rajesh Taneja added a comment - Some string related functions in lib/moodlelib.php can be moved to textlib.
              Hide
              skodak Petr Skoda added a comment -

              thanks for the report, we did not do it earlier because we cared about potential backporting issues - it is no longer a problem because the textlib:: is allowed in stable supported versions

              Show
              skodak Petr Skoda added a comment - thanks for the report, we did not do it earlier because we cared about potential backporting issues - it is no longer a problem because the textlib:: is allowed in stable supported versions
              Hide
              skodak Petr Skoda added a comment -

              I suppose the moving of string related functions to textlib should be done in separate issue because it would be changes in API, this issue is cosmetic code cleanup only.

              Show
              skodak Petr Skoda added a comment - I suppose the moving of string related functions to textlib should be done in separate issue because it would be changes in API, this issue is cosmetic code cleanup only.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Note: static is only allowed in 22_STABLE and master, not 21_STABLE. So we'll need to be super careful with 21_STABLE.

              Some little details:

              • Shouldn't textlib_get_instance() go to deprecatedlib + debugging message?
              • Deprecated ones must be documented @ lib/upgrade.txt too

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Note: static is only allowed in 22_STABLE and master, not 21_STABLE. So we'll need to be super careful with 21_STABLE. Some little details: Shouldn't textlib_get_instance() go to deprecatedlib + debugging message? Deprecated ones must be documented @ lib/upgrade.txt too Ciao
              Hide
              rajeshtaneja Rajesh Taneja added a comment -

              Thanks for fixing this Petr
              @Eloy:
              I am not sure about deprecatedlib. Last I heard about deprecatedlib was, it was created for deprecating 1.9 api's.

              Show
              rajeshtaneja Rajesh Taneja added a comment - Thanks for fixing this Petr @Eloy: I am not sure about deprecatedlib. Last I heard about deprecatedlib was, it was created for deprecating 1.9 api's.
              Hide
              stronk7 Eloy Lafuente (stronk7) 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
              stronk7 Eloy Lafuente (stronk7) 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
              stronk7 Eloy Lafuente (stronk7) added a comment -

              I'm reopening, no feedback for a week, surely not under the radar of the developer...

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - I'm reopening, no feedback for a week, surely not under the radar of the developer...
              Hide
              skodak Petr Skoda added a comment -

              hi, I have rebased it and moved the textlib_get_instance() to deprecatedlib.

              Show
              skodak Petr Skoda added a comment - hi, I have rebased it and moved the textlib_get_instance() to deprecatedlib.
              Hide
              stronk7 Eloy Lafuente (stronk7) 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
              stronk7 Eloy Lafuente (stronk7) 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
              skodak Petr Skoda added a comment -

              rebased

              Show
              skodak Petr Skoda added a comment - rebased
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Integrated, thanks!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Passing, install/upgrade and unit test passed in the CI server, plus manual review of changes looked perfect.

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Passing, install/upgrade and unit test passed in the CI server, plus manual review of changes looked perfect.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!).

              icao_reverse('arreis olik rebemevon afla letoh ognat');

              Closing, ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!). icao_reverse('arreis olik rebemevon afla letoh ognat'); Closing, ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    25/Jun/12