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

Delete forum_get_separate_modules() function.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Forum
    • Labels:

      Description

      While reviewing MDL-33166, the forum_get_separate_modules() was discovered.

      1) It's 100% wrong:

      • comparing cm.module with forum.id (should be cm.instance)
      • missing join with modules table to look only within forums.

      2) It's not used at all in master codebase (nor in older branches), nor in contrib.

      So, this is, simply, about to take rid of that function in master once and forever.

      No need to document it nor to test anything. Simple delete.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Sending to integration.

            I don't think this requires api_changes documentation o similar. No way that function was working at all since 2.0 (where the mess happenend @ 4e4453552618fbc7cfecbe77584d8d158856c46d), and I've not found any use in core nor contrib.

            So surely, deleting it silently for master is enough.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Sending to integration. I don't think this requires api_changes documentation o similar. No way that function was working at all since 2.0 (where the mess happenend @ 4e4453552618fbc7cfecbe77584d8d158856c46d), and I've not found any use in core nor contrib. So surely, deleting it silently for master is enough.
            Hide
            poltawski 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
            poltawski 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
            poltawski Dan Poltawski added a comment -

            I've integrated this now. Even though it was completely useless I think we could add an upgrade.txt as a courtesy (even to warn that it was previously returning incorrect results).

            But I don't think its critical, so i've integrated this

            Show
            poltawski Dan Poltawski added a comment - I've integrated this now. Even though it was completely useless I think we could add an upgrade.txt as a courtesy (even to warn that it was previously returning incorrect results). But I don't think its critical, so i've integrated this
            Hide
            poltawski Dan Poltawski added a comment -

            Congratulations, you've done it!

            Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc

            Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.

            Show
            poltawski Dan Poltawski added a comment - Congratulations, you've done it! Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  3/Dec/12