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

New plugins for mod_assign should use naming convention for constants

    Details

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

      Description

      e.g. ASSIGN_MAX_SUBMISSION_FILES should be ASSIGNSUBMISSION_FILE_MAXFILES

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            damyon Damyon Wiese added a comment -

            Reported by Petr Škoda

            Show
            damyon Damyon Wiese added a comment - Reported by Petr Škoda
            Hide
            dougiamas Martin Dougiamas added a comment -

            I've made you component lead now, Damyon, so these will be assigned to you by default.

            Show
            dougiamas Martin Dougiamas added a comment - I've made you component lead now, Damyon, so these will be assigned to you by default.
            Hide
            damyon Damyon Wiese added a comment -

            I have cherry-picked this change onto my master branch and updated the git diff url.

            Regards, Damyon

            Show
            damyon Damyon Wiese added a comment - I have cherry-picked this change onto my master branch and updated the git diff url. Regards, Damyon
            Hide
            damyon Damyon Wiese added a comment -

            Moved to clean branch based off Moodle master.

            Show
            damyon Damyon Wiese added a comment - Moved to clean branch based off Moodle master.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Damyon,

            Changes look good thanks.
            In future can you please create branches each issue you are working on. It makes it much easier for us to see what is going on, and it makes integration a MUCH easier task.

            For the integrator:

            git fetch git://github.com/netspotau/moodle-mod_assign.git master; git cherry-pick e8df13491d3cb3f1eb4d884d2675878186b96b6d

            Putting this up for integration immediately (code freeze today).

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Damyon, Changes look good thanks. In future can you please create branches each issue you are working on. It makes it much easier for us to see what is going on, and it makes integration a MUCH easier task. For the integrator: git fetch git://github.com/netspotau/moodle-mod_assign.git master; git cherry-pick e8df13491d3cb3f1eb4d884d2675878186b96b6d Putting this up for integration immediately (code freeze today). Cheers Sam
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated - thanks.

            Also thanks for doing the branching so quickly!

            Show
            poltawski Dan Poltawski added a comment - Integrated - thanks. Also thanks for doing the branching so quickly!
            Hide
            poltawski Dan Poltawski added a comment -

            Failing test as I found a use of the plugin constant which has been removed:

            git grep ASSIGN_FILEAREA_SUBMISSION_FILES
            mod/assign/locallib.php: private function count_files($userid = 0, $area = ASSIGN_FILEAREA_SUBMISSION_FILES) {

            Note that the solution here can't just be to replace that constant to the new one since there is no depdency relationship between the two.

            Show
            poltawski Dan Poltawski added a comment - Failing test as I found a use of the plugin constant which has been removed: git grep ASSIGN_FILEAREA_SUBMISSION_FILES mod/assign/locallib.php: private function count_files($userid = 0, $area = ASSIGN_FILEAREA_SUBMISSION_FILES) { Note that the solution here can't just be to replace that constant to the new one since there is no depdency relationship between the two.
            Hide
            damyon Damyon Wiese added a comment -

            Hi Dan,

            I did a search and that function is not used anywhere so I have removed it. The plugins implement their own count_files functions that know where the plugin file areas are.

            I pushed this as a separate commit - let me know if you want me to squash it. I updated the git diff url. The two commits are:

            8ab7bb9f5620cb2217f5bec666b47cd9a7ca324d
            and
            3ec209133d99be31ab53dbed41ba5b87c2cd764c

            (oldest to newest).

            Regards, Damyon

            Show
            damyon Damyon Wiese added a comment - Hi Dan, I did a search and that function is not used anywhere so I have removed it. The plugins implement their own count_files functions that know where the plugin file areas are. I pushed this as a separate commit - let me know if you want me to squash it. I updated the git diff url. The two commits are: 8ab7bb9f5620cb2217f5bec666b47cd9a7ca324d and 3ec209133d99be31ab53dbed41ba5b87c2cd764c (oldest to newest). Regards, Damyon
            Hide
            damyon Damyon Wiese added a comment -

            Also - I'm not sure if I am meant to do something with the status of this ticket now it is in "Problem during testing" or if that is for Sam to do.

            Show
            damyon Damyon Wiese added a comment - Also - I'm not sure if I am meant to do something with the status of this ticket now it is in "Problem during testing" or if that is for Sam to do.
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Damyon, that was perfect.

            When we find a problem in testing the outcome is either to pull in a fix for the problem, or revert the change.

            I've pulled in your change now, so this is restarted for testing again

            Show
            poltawski Dan Poltawski added a comment - Thanks Damyon, that was perfect. When we find a problem in testing the outcome is either to pull in a fix for the problem, or revert the change. I've pulled in your change now, so this is restarted for testing again
            Hide
            poltawski Dan Poltawski added a comment -

            Passing, thanks!

            Show
            poltawski Dan Poltawski added a comment - Passing, thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            This is now part of Moodle and a few millions people around the globe will be using it soon. Isn't that awesome?

            Many, many thanks and don't forget http://youtu.be/4N7dPaP5Z8U

            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - This is now part of Moodle and a few millions people around the globe will be using it soon. Isn't that awesome? Many, many thanks and don't forget http://youtu.be/4N7dPaP5Z8U Closing, ciao

              People

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

                Dates

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