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:
    • Rank:
      42370

      Description

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

        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: