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

      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.

        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: