Details

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

      Gliffy Diagrams

        Attachments

          Activity

          Hide
          stronk7 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
          stronk7 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
          skodak Petr Skoda 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
          skodak Petr Skoda 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
          skodak Petr Skoda added a comment -

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

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

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

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

          Integrated (master only), thanks!

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

          Thanks Petr,

          Works Great.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Thanks Petr, Works Great.
          Hide
          stronk7 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
          stronk7 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:
                Fix Release Date:
                3/Dec/12