Moodle
  1. Moodle
  2. MDL-31147 META: Collect together deprecated code changes for Moodle 2.3
  3. MDL-32942

Deprecated PARAM_ACTION and PARAM_FORMAT should be replaced with PARAM_ALPHANUMEXT

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Libraries
    • Labels:
    • Testing Instructions:
      Hide

      Test 1:
      grep -Ri PARAM_ACTION .
      grep -Ri PARAM_FORMAT .
      and make sure it's not used in code. Only declaration is there in lib/moodlelib.php

      Test 2:
      Review changes and try find any silly mistake.

      Show
      Test 1: grep -Ri PARAM_ACTION . grep -Ri PARAM_FORMAT . and make sure it's not used in code. Only declaration is there in lib/moodlelib.php Test 2: Review changes and try find any silly mistake.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      wip-mdl-32942

      Description

      PARAM_ACTION and PARAM_FORMAT were deprecated in 2.0, please replace it with PARAM_ALPHANUMEXT and remove PARAM_ACTION and PARAM_FORMAT declaration.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Rajesh Taneja added a comment -

            Not removed declaration for 3rd party compatibility reasons.

            Show
            Rajesh Taneja added a comment - Not removed declaration for 3rd party compatibility reasons.
            Hide
            Sam Hemelryk added a comment -

            Hi Raj,

            Changes look good thanks, feel free to put this up for integration when ready.

            There are two thoughts I've had while looking at these changes.

            1. We should consider moving the deprecated params to deprecatedlib.php and giving them all an @since version.
            2. We should add a unit test to tests/moodlelib_test.php to test all deprecated PARAM_* are matching to the expected type (just to make sure no-one makes further changes and to be truely aware when removing them for good)

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Hi Raj, Changes look good thanks, feel free to put this up for integration when ready. There are two thoughts I've had while looking at these changes. We should consider moving the deprecated params to deprecatedlib.php and giving them all an @since version. We should add a unit test to tests/moodlelib_test.php to test all deprecated PARAM_* are matching to the expected type (just to make sure no-one makes further changes and to be truely aware when removing them for good) Cheers Sam
            Hide
            Rajesh Taneja added a comment -

            Thanks Sam,

            Is it fine to open a new bug to move deprecated params and add test case or shall I do it as part of this bug?

            Show
            Rajesh Taneja added a comment - Thanks Sam, Is it fine to open a new bug to move deprecated params and add test case or shall I do it as part of this bug?
            Hide
            Sam Hemelryk added a comment -

            Probably best to handle it as a separate bug, and I'd both move the params and create test cases at the same time.
            Before you do however check with Petr, just to be sure there is no good reason to keep them in the current location. I think moving them is a good idea that way when people locate them to check how they work they will find them in deprecated lib and be less tempted to use them.

            Show
            Sam Hemelryk added a comment - Probably best to handle it as a separate bug, and I'd both move the params and create test cases at the same time. Before you do however check with Petr, just to be sure there is no good reason to keep them in the current location. I think moving them is a good idea that way when people locate them to check how they work they will find them in deprecated lib and be less tempted to use them.
            Hide
            Rajesh Taneja added a comment -

            Thanks Sam.

            Show
            Rajesh Taneja added a comment - Thanks Sam.
            Hide
            Dan Poltawski added a comment -

            Delaying till the start of the 2.4 dev cycle. We don't want any more big changes for now.

            Show
            Dan Poltawski added a comment - Delaying till the start of the 2.4 dev cycle. We don't want any more big changes for now.
            Hide
            Rajesh Taneja added a comment -

            Branch re-based
            Pushing for integration review.

            Show
            Rajesh Taneja added a comment - Branch re-based Pushing for integration review.
            Hide
            Dan Poltawski 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
            Dan Poltawski 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
            Rajesh Taneja added a comment -

            Branch re-based.

            Show
            Rajesh Taneja added a comment - Branch re-based.
            Hide
            Aparup Banerjee added a comment -

            Hi Raj, i've also noticed PARAM_MULTILANG and PARAM_CLEANFILE marked as deprecated. (seems since 2.0 , 2009-ish) i'm not sure if there are issues for these , but they could all be linked up somewhere.

            Show
            Aparup Banerjee added a comment - Hi Raj, i've also noticed PARAM_MULTILANG and PARAM_CLEANFILE marked as deprecated. (seems since 2.0 , 2009-ish) i'm not sure if there are issues for these , but they could all be linked up somewhere.
            Hide
            Aparup Banerjee added a comment -

            Thanks Raj, thats integrated into master now.

            ps: i can't see that any docs would require updating.

            pps (Tester): a real tester would run through all the changed pages and verify that the params still work fine. That would be a large test though. your choice is : a developer's hat or a tester's hat? :-p

            Show
            Aparup Banerjee added a comment - Thanks Raj, thats integrated into master now. ps: i can't see that any docs would require updating. pps (Tester): a real tester would run through all the changed pages and verify that the params still work fine. That would be a large test though. your choice is : a developer's hat or a tester's hat? :-p
            Hide
            Ankit Agarwal added a comment -

            Developer's hat cause am a developer

            Everything looks good to me.

            Thanks

            Show
            Ankit Agarwal added a comment - Developer's hat cause am a developer Everything looks good to me. 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: