Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Libraries
    • Labels:
    • Rank:
      43826

      Activity

      Hide
      Eloy Lafuente (stronk7) added a comment -

      Uhm, it seems that from the 2 changes applied for previous version on 29329a251933d697bca8925ed5a8bc1242acc8fe, the getURL() => getUrl() one is not needed anymore (unless you've applied it).

      But the empty check on initCharset() seems to be missing in your proposed patch.

      So it requires clarification @ readme_moodle.txt about what exactly has been changed from upstream and also... add the missing empty check commented above.

      I'd do it but I don't know if you've applied any other change already, sorry. Please review.

      Ciao

      Show
      Eloy Lafuente (stronk7) added a comment - Uhm, it seems that from the 2 changes applied for previous version on 29329a251933d697bca8925ed5a8bc1242acc8fe, the getURL() => getUrl() one is not needed anymore (unless you've applied it). But the empty check on initCharset() seems to be missing in your proposed patch. So it requires clarification @ readme_moodle.txt about what exactly has been changed from upstream and also... add the missing empty check commented above. I'd do it but I don't know if you've applied any other change already, sorry. Please review. Ciao
      Hide
      Petr Škoda added a comment -

      The readme says:

      Changes: none
      

      which is correct, I did not modify it at all and it works fine in unittests, that proves those changes are not necessary any more.

      Show
      Petr Škoda added a comment - The readme says: Changes: none which is correct, I did not modify it at all and it works fine in unittests, that proves those changes are not necessary any more.
      Hide
      Petr Škoda added a comment -

      The test coverage of code using typo3 should be already 100%, I was adding test for any potential problems before.

      Show
      Petr Škoda added a comment - The test coverage of code using typo3 should be already 100%, I was adding test for any potential problems before.
      Hide
      Eloy Lafuente (stronk7) added a comment -

      Oki, just hope those old notices won't appear again suddenly. Integrating...

      Show
      Eloy Lafuente (stronk7) added a comment - Oki, just hope those old notices won't appear again suddenly. Integrating...
      Hide
      Eloy Lafuente (stronk7) added a comment -

      Integrated (master only), thanks!

      Show
      Eloy Lafuente (stronk7) added a comment - Integrated (master only), thanks!
      Hide
      Rajesh Taneja added a comment -

      Thanks Petr,

      Works Great.

      Show
      Rajesh Taneja added a comment - Thanks Petr, Works Great.
      Hide
      Eloy Lafuente (stronk7) added a comment -

      Many thanks for the hard work.

      These changes have been spread upstream and are already available in the git and cvs repositories.

      Ciao

      Show
      Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work. These changes have been spread upstream and are already available in the git and cvs repositories. Ciao

        People

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

          Dates

          • Created:
            Updated:
            Resolved: