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

The use of $settings in add_admin_assign_plugin_settings is weird

    Details

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

      Description

      As far I can tell $settings is not being [used] as its not being passed by reference (which is a good thing IMO)

      So the whole $tempsettings thing around the loop isn't required?

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            poltawski Dan Poltawski added a comment -

            Looks good, submitting for integration - thanks!

            Show
            poltawski Dan Poltawski added a comment - Looks good, submitting for integration - thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) 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
            stronk7 Eloy Lafuente (stronk7) 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
            damyon Damyon Wiese added a comment - - edited

            I have just rebased this to the latest weekly dev.

            Show
            damyon Damyon Wiese added a comment - - edited I have just rebased this to the latest weekly dev.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Damyon,
            Looks like you've got the commit for MDL-32933 here in the MDL-32903 branch.
            I've peer-reviewed MDL-32933 now for you, but I couldn't find the MDL-32903 commit sorry.
            Could you please have a look and find it and ping me with a location so I can review it.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Damyon, Looks like you've got the commit for MDL-32933 here in the MDL-32903 branch. I've peer-reviewed MDL-32933 now for you, but I couldn't find the MDL-32903 commit sorry. Could you please have a look and find it and ping me with a location so I can review it. Cheers Sam
            Hide
            damyon Damyon Wiese added a comment -

            Sorry - I made a typo when I rebased it. All fixed now.

            Show
            damyon Damyon Wiese added a comment - Sorry - I made a typo when I rebased it. All fixed now.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Damyon, changes look sensible and this has been integrated now

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Damyon, changes look sensible and this has been integrated now
            Hide
            andyjdavis Andrew Davis added a comment -

            Looks good. Passing.

            Show
            andyjdavis Andrew Davis added a comment - Looks good. Passing.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            U P S T R E A M I Z E D !

            Many thanks for the hard work, closing this as fixed.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks for the hard work, closing this as fixed. Ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Jun/12