Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3.1
    • Component/s: Libraries
    • Labels:
    • Testing Instructions:
      Hide

      Developer thing only. I don't think it needs to be tested. If you really want to test it, then review the patch and work out what needs to be done yourself.

      Show
      Developer thing only. I don't think it needs to be tested. If you really want to test it, then review the patch and work out what needs to be done yourself.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Working on some unit tests, these two just confused me.

        Gliffy Diagrams

          Activity

          Hide
          Tim Hunt added a comment -

          Since this is a developer thing, I am assuming master only. If you want to cherry-pick, however, please be my guest.

          Show
          Tim Hunt added a comment - Since this is a developer thing, I am assuming master only. If you want to cherry-pick, however, please be my guest.
          Hide
          Andrew Nicols added a comment -

          Typo in the string: indentifier

          Show
          Andrew Nicols added a comment - Typo in the string: indentifier
          Hide
          Tim Hunt added a comment -

          Thanks Andrew. Amended.

          Also, I am cheating, and using this branch to fix two typos in comments in the question engine that I noticed today.

          Show
          Tim Hunt added a comment - Thanks Andrew. Amended. Also, I am cheating, and using this branch to fix two typos in comments in the question engine that I noticed today.
          Hide
          Dan Poltawski added a comment -

          Taking integration held issues out of integration (whilst we are keeping master and 23_STABLE in sync).

          Show
          Dan Poltawski added a comment - Taking integration held issues out of integration (whilst we are keeping master and 23_STABLE in sync).
          Hide
          Tim Hunt added a comment -

          It is a pretty safe change. Are you sure you don't want to cherry-pick it?

          Show
          Tim Hunt added a comment - It is a pretty safe change. Are you sure you don't want to cherry-pick it?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Just amended the commit to change 2 occurrences of "accordin" to "according".

          BTW the 2 methods having them have 100% the same phpdocs (feedback and hints). Perhaps it's copy/paste mistake? For your consideration...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Just amended the commit to change 2 occurrences of "accordin" to "according". BTW the 2 methods having them have 100% the same phpdocs (feedback and hints). Perhaps it's copy/paste mistake? For your consideration...ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks! (23 and master)

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks! (23 and master)
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Passed (under my resp).

          Show
          Eloy Lafuente (stronk7) added a comment - Passed (under my resp).
          Hide
          Paul Holden added a comment -

          Is the change to the exception message thrown by get_string correct? The $identifier is passed through clean_param just prior to it, so it could be empty because it didn't match the PARAM_STRINGID rule... not because it was never passed to the method

          Show
          Paul Holden added a comment - Is the change to the exception message thrown by get_string correct? The $identifier is passed through clean_param just prior to it, so it could be empty because it didn't match the PARAM_STRINGID rule... not because it was never passed to the method
          Hide
          Tim Hunt added a comment -

          Oh. Well the case where I encountered this, it was empty, and so the error message completely confused me. That is why I changed it. You are right that it is still not 100% accurate.

          Show
          Tim Hunt added a comment - Oh. Well the case where I encountered this, it was empty, and so the error message completely confused me. That is why I changed it. You are right that it is still not 100% accurate.
          Hide
          Sam Hemelryk added a comment -

          Congratulations your code is upstream - gold star for you!

          This issue + 79 others made it in in time for the minor releases.
          Thank you everyone involved for your exuberant efforts.

          Show
          Sam Hemelryk added a comment - Congratulations your code is upstream - gold star for you! This issue + 79 others made it in in time for the minor releases. Thank you everyone involved for your exuberant efforts.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: