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

Strings imsroleadmin and imsroleinstructor are unused

    Details

      Description

      mod/lti/lang/en/lti.php contains references to imsroleadmin and imsroleinstructor which do not appear to be used anywhere. If this is in fact the case we should remove them in 2.3.

      It is probably a bad idea to remove them in current versions as this is more of an improvement than a fix and there is the small possibility that somebody could be calling these in code external to Moodle.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              salvetore Michael de Raadt added a comment -

              Up to you if you want to do this or close this issue, Chris.

              Show
              salvetore Michael de Raadt added a comment - Up to you if you want to do this or close this issue, Chris.
              Hide
              gerry Gerard Caulfield added a comment -

              Just so it's clear, I was requested to create this issue here: http://tracker.moodle.org/browse/MDL-30935?focusedCommentId=144618#comment-144618

              Show
              gerry Gerard Caulfield added a comment - Just so it's clear, I was requested to create this issue here: http://tracker.moodle.org/browse/MDL-30935?focusedCommentId=144618#comment-144618
              Hide
              ralfh Ralf Hilgenstock added a comment -

              Can you please also check the [setdefault,mod_lti]. Its using the term professor, that is never used in Moodle. Please replace by teacher.

              Show
              ralfh Ralf Hilgenstock added a comment - Can you please also check the [setdefault,mod_lti] . Its using the term professor, that is never used in Moodle. Please replace by teacher.
              Hide
              scriby Chris Scribner added a comment -

              Fix is here:

              https://github.com/scriby/moodle/commits/MDL-31657

              I removed a couple other strings nearby that also weren't in use. I suspect there are a number more in that file that aren't used.

              Testing instructions:

              I did a code grep to make sure the localization strings weren't in use. Just running through the LTI screens and confirming it's not trying to use one of the localization strings is all I can think of on testing.

              Show
              scriby Chris Scribner added a comment - Fix is here: https://github.com/scriby/moodle/commits/MDL-31657 I removed a couple other strings nearby that also weren't in use. I suspect there are a number more in that file that aren't used. Testing instructions: I did a code grep to make sure the localization strings weren't in use. Just running through the LTI screens and confirming it's not trying to use one of the localization strings is all I can think of on testing.
              Hide
              poltawski Dan Poltawski added a comment -

              Looks good:

              (M=cf487 ?3) danp@marge ~/git/integration> git grep imsroleadmin
              mod/lti/lang/en/lti.php:$string['imsroleadmin'] = 'Instructor,Administrator';
              (M=cf487 ?3) danp@marge ~/git/integration> git grep imsroleinstructor
              mod/lti/lang/en/lti.php:$string['imsroleinstructor'] = 'Instructor';
              (M=cf487 ?3) danp@marge ~/git/integration> git grep imsrolelearner
              mod/lti/lang/en/lti.php:$string['imsrolelearner'] = 'Learner';
              (M=cf487 ?3) danp@marge ~/git/integration> git grep setupbox | grep lti
              mod/lti/lang/en/lti.php:$string['setupbox'] = 'LTI Tool Setup Box';
              (M=cf487 ?3) danp@marge ~/git/integration> git grep setdefault | grep lti
              mod/lti/lang/en/lti.php:$string['setdefault'] = 'Set a default value for the professor if delegating';

              Show
              poltawski Dan Poltawski added a comment - Looks good: (M=cf487 ?3) danp@marge ~/git/integration> git grep imsroleadmin mod/lti/lang/en/lti.php:$string['imsroleadmin'] = 'Instructor,Administrator'; (M=cf487 ?3) danp@marge ~/git/integration> git grep imsroleinstructor mod/lti/lang/en/lti.php:$string['imsroleinstructor'] = 'Instructor'; (M=cf487 ?3) danp@marge ~/git/integration> git grep imsrolelearner mod/lti/lang/en/lti.php:$string['imsrolelearner'] = 'Learner'; (M=cf487 ?3) danp@marge ~/git/integration> git grep setupbox | grep lti mod/lti/lang/en/lti.php:$string['setupbox'] = 'LTI Tool Setup Box'; (M=cf487 ?3) danp@marge ~/git/integration> git grep setdefault | grep lti mod/lti/lang/en/lti.php:$string['setdefault'] = 'Set a default value for the professor if delegating';
              Hide
              poltawski Dan Poltawski added a comment -

              I'm submitting thsi for integration now, thanks!

              Chris: it'd be great if you could fill out the git repository/branch/diff fields when submititng these as its necessary for submitting to integration.

              Thanks!

              Show
              poltawski Dan Poltawski added a comment - I'm submitting thsi for integration now, thanks! Chris: it'd be great if you could fill out the git repository/branch/diff fields when submititng these as its necessary for submitting to 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
              nebgor Aparup Banerjee added a comment -

              cool, integrated into master.

              Show
              nebgor Aparup Banerjee added a comment - cool, integrated into master.
              Hide
              abgreeve Adrian Greeve added a comment -

              No testing instructions present. I did a grep for the different strings in the code to see if it was being called from anywhere. The result turned up negative.
              Passing.
              Thanks.

              Show
              abgreeve Adrian Greeve added a comment - No testing instructions present. I did a grep for the different strings in the code to see if it was being called from anywhere. The result turned up negative. Passing. Thanks.
              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:
                  0 Start watching this issue

                  Dates

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