Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 2.4
    • Fix Version/s: STABLE backlog
    • Component/s: Libraries
    • Labels:
    • Affected Branches:
      MOODLE_24_STABLE
    • Rank:
      42981

      Description

      The function was deprecated in 2.1 and removed from core with a deprecated warning in 2.4
      Refer MDL-33061 for further details.

      It should be safe to remove the function from core in 2.6

      Thanks

        Issue Links

          Activity

          Hide
          Ankit Agarwal added a comment -

          We might have to move this to a later version depending on when it is decided to integrate MDL-34472

          Thanks

          Show
          Ankit Agarwal added a comment - We might have to move this to a later version depending on when it is decided to integrate MDL-34472 Thanks
          Hide
          Martin Dougiamas added a comment -

          I don't think there is a good case to remove this from core totally. As long as we have DEVELOPER warnings to help them upgrade to the new ones there is no reason to remove this function completely as long as it works.

          Show
          Martin Dougiamas added a comment - I don't think there is a good case to remove this from core totally. As long as we have DEVELOPER warnings to help them upgrade to the new ones there is no reason to remove this function completely as long as it works.
          Hide
          Ankit Agarwal added a comment -

          We decided never to remove this from core. It will always stay with the debugging notice.
          closing this issue as won't fix.

          Show
          Ankit Agarwal added a comment - We decided never to remove this from core. It will always stay with the debugging notice. closing this issue as won't fix.
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          While I agree with Dan's main points @ MDL-34472:

          • there are many uses of it by non-core
          • codechecker should be able to look for them.
          • we cannot delete it "suddenly".
          • ....

          I simply think that "deprecating forever" is not a good principle. I don't mind if it's Moodle 3.0, or Moodle 2.112. But deprecation must have an end date/release.

          Surely it's our fault that we missed to document those deprecations properly in 2.2, but well, you know how the initial 2.x cycles were.

          I really take this as a second opportunity do the things properly, with "official" deprecation of this function (and others) with documentation, checkers and a target version for removal (no matter how distant it is).

          But "deprecating forever", no. Sorry my -99 for that.

          Ciao

          Edited: 2.2

          Show
          Eloy Lafuente (stronk7) added a comment - - edited While I agree with Dan's main points @ MDL-34472 : there are many uses of it by non-core codechecker should be able to look for them. we cannot delete it "suddenly". .... I simply think that "deprecating forever" is not a good principle. I don't mind if it's Moodle 3.0, or Moodle 2.112. But deprecation must have an end date/release. Surely it's our fault that we missed to document those deprecations properly in 2.2, but well, you know how the initial 2.x cycles were. I really take this as a second opportunity do the things properly, with "official" deprecation of this function (and others) with documentation, checkers and a target version for removal (no matter how distant it is). But "deprecating forever", no. Sorry my -99 for that. Ciao Edited: 2.2
          Hide
          Dan Poltawski added a comment -

          Well, if you want to frame it another way, I don't think these accessors should've been deprecated and should continue to be fully supported.

          What would change my mind is if someone could answer this question with something meaningful:

          > The benefit of changing every piece of code to use context_module::instance() over get_context_instance(CONTEXT_MODULE,) is?

          The way it looks to me is:

          1. Developer creates newer (slightly nicer) API
          2. Developer deprecates old way (with no clear benefit, or discussion)
          3. Whole community has to do a lot of unnecessary work to update to new style

          (In the middle of writing this, Damyon and I had a discussion about consistency for new developers, being tied down by legacy, which sure mellowed me whilst posting this )

          Show
          Dan Poltawski added a comment - Well, if you want to frame it another way, I don't think these accessors should've been deprecated and should continue to be fully supported. What would change my mind is if someone could answer this question with something meaningful: > The benefit of changing every piece of code to use context_module::instance() over get_context_instance(CONTEXT_MODULE,) is? The way it looks to me is: Developer creates newer (slightly nicer) API Developer deprecates old way (with no clear benefit, or discussion) Whole community has to do a lot of unnecessary work to update to new style (In the middle of writing this, Damyon and I had a discussion about consistency for new developers, being tied down by legacy, which sure mellowed me whilst posting this )
          Hide
          Damyon Wiese added a comment -

          IMO - we should be safe to remove deprecations when all supported versions of Moodle include the new function (This is longer than the currently documented period). This means plugin developers can use one function for all supported versions of Moodle.

          Show
          Damyon Wiese added a comment - IMO - we should be safe to remove deprecations when all supported versions of Moodle include the new function (This is longer than the currently documented period). This means plugin developers can use one function for all supported versions of Moodle.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: