Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.4
    • Component/s: Libraries
    • Labels:

      Description

      There are dozens of functions and classes that have been deprecated since Moodle 2.0 within lib/pagelib.php that should be removed shortly.
      I am thinking 2.3 would be a good time to remove these classes. Certainly given 2.0 is no longer supported it is safe to do it now.

      I will address this after MDL-30977 has been integrated.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Rajesh Taneja added a comment -

            Hello Sam,

            Is it safe to remove magic_get_legacythemeinuse as well?
            It is not used anywhere.

            Show
            Rajesh Taneja added a comment - Hello Sam, Is it safe to remove magic_get_legacythemeinuse as well? It is not used anywhere.
            Hide
            Sam Hemelryk added a comment -

            Hi Raj,
            I've looked through these now and made the following notes.
            Still a bit of work sorry.

            • deprecatedlib.php::page_id_and_class either needs to be removed or a strict exception thrown. That method is currently broken but tries to use two now removed properties pagetype and legacyclass.
            • blocks_get_default also has a use in core still... don't know how that works you'll have to look into it but I'd bet its flat out broken and needs review. Should be cleaned up at the same time for sure.
            • I think it is safe to remove magic_get_legacythemeinuse as well.
            • String debug=>noactivityname can be removed now (clean up now while we know whats going on).
            • You need to check all of the properties for the moodle_page class. They wern't marked as deprecated obviously but there were several used by now removed methods that are not used anywhere else. They need to be cleaned up. Check out legacypageobject for a good place to start.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Hi Raj, I've looked through these now and made the following notes. Still a bit of work sorry. deprecatedlib.php::page_id_and_class either needs to be removed or a strict exception thrown. That method is currently broken but tries to use two now removed properties pagetype and legacyclass. blocks_get_default also has a use in core still... don't know how that works you'll have to look into it but I'd bet its flat out broken and needs review. Should be cleaned up at the same time for sure. I think it is safe to remove magic_get_legacythemeinuse as well. String debug=>noactivityname can be removed now (clean up now while we know whats going on). You need to check all of the properties for the moodle_page class. They wern't marked as deprecated obviously but there were several used by now removed methods that are not used anywhere else. They need to be cleaned up. Check out legacypageobject for a good place to start. Cheers Sam
            Hide
            Rajesh Taneja added a comment -

            Thanks Sam,

            Will take this up after 2.3 release.

            Show
            Rajesh Taneja added a comment - Thanks Sam, Will take this up after 2.3 release.
            Hide
            Rajesh Taneja added a comment -

            Thanks for the pointers Sam,

            I have integrated all your points. I checked moodle_page class and don't see anything except legacypageobject which is removed. IMO legacyclass is still in use, please suggest if I am wrong.

            Once again up for your review.

            Show
            Rajesh Taneja added a comment - Thanks for the pointers Sam, I have integrated all your points. I checked moodle_page class and don't see anything except legacypageobject which is removed. IMO legacyclass is still in use, please suggest if I am wrong. Once again up for your review.
            Hide
            Sam Hemelryk added a comment -

            Changes look good thanks Raj, couldn't find any other properties that were no longer used.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Changes look good thanks Raj, couldn't find any other properties that were no longer used. Cheers Sam
            Hide
            Rajesh Taneja added a comment -

            Thanks Sam,

            Pushing for integration review.

            Show
            Rajesh Taneja added a comment - Thanks Sam, Pushing for integration review.
            Hide
            Dan Poltawski added a comment -

            Thanks Raj, i've integrated this now.

            Its a shame that get_format_name wasn't throwing a debugging notice but I am going to let this slide for the good of this change (and it pretty legacy stuff).

            I added a commit on top of your changes to document this deprecations in lib/upgrade.txt

            Show
            Dan Poltawski added a comment - Thanks Raj, i've integrated this now. Its a shame that get_format_name wasn't throwing a debugging notice but I am going to let this slide for the good of this change (and it pretty legacy stuff). I added a commit on top of your changes to document this deprecations in lib/upgrade.txt
            Hide
            Rajesh Taneja added a comment -

            Thanks Dan.

            Show
            Rajesh Taneja added a comment - Thanks Dan.
            Hide
            Ankit Agarwal added a comment -

            Tested some random things, including changing of themes.
            All looks good.
            Thanks

            Show
            Ankit Agarwal added a comment - Tested some random things, including changing of themes. All looks good. Thanks
            Hide
            Aparup Banerjee added a comment -

            yay, it works!

            This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week.

            Thank you all for taking the time to get us here.

            cheers!

            Show
            Aparup Banerjee added a comment - yay, it works! This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week. Thank you all for taking the time to get us here. cheers!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: