Moodle
  1. Moodle
  2. MDL-32768

New plugins for mod_assign should use naming convention for constants

    Details

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

      Description

      e.g. ASSIGN_MAX_SUBMISSION_FILES should be ASSIGNSUBMISSION_FILE_MAXFILES

        Issue Links

          Activity

          Hide
          Damyon Wiese added a comment -

          Reported by Petr Škoda

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

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

          Show
          Martin Dougiamas added a comment - I've made you component lead now, Damyon, so these will be assigned to you by default.
          Hide
          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 Wiese added a comment - I have cherry-picked this change onto my master branch and updated the git diff url. Regards, Damyon
          Hide
          Damyon Wiese added a comment -

          Moved to clean branch based off Moodle master.

          Show
          Damyon Wiese added a comment - Moved to clean branch based off Moodle master.
          Hide
          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
          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
          Dan Poltawski added a comment -

          Integrated - thanks.

          Also thanks for doing the branching so quickly!

          Show
          Dan Poltawski added a comment - Integrated - thanks. Also thanks for doing the branching so quickly!
          Hide
          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
          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 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 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 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 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
          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
          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
          Dan Poltawski added a comment -

          Passing, thanks!

          Show
          Dan Poltawski added a comment - Passing, thanks!
          Hide
          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
          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: