Moodle
  1. Moodle
  2. MDL-30980

string API, check and update DocBlock

    Details

      Description

      Check and update documentation, so that it should comply with moodle coding guidelines.
      Following needs to be updated/checked for string api

      1. DocBlock for page and functions.
      2. All the files should be checked/updated.

      Note: You can create sub-tasks, so as to avoid bulk integration.

        Gliffy Diagrams

          Activity

          Hide
          Martin Dougiamas added a comment -

          Main things that need nice phpdocs and '@category string" are only get_string, get_strings, print_string.

          Describe them on http://docs.moodle.org/dev/String_API

          Show
          Martin Dougiamas added a comment - Main things that need nice phpdocs and '@category string" are only get_string, get_strings, print_string. Describe them on http://docs.moodle.org/dev/String_API
          Hide
          Rajesh Taneja added a comment -

          Taking this one, as discussed with Martin.

          Show
          Rajesh Taneja added a comment - Taking this one, as discussed with Martin.
          Hide
          Sam Hemelryk added a comment -

          Raj, don't forget the lang_string class that has been introduced in Moodle 2.3
          No documentation at all for that, I did document the code but I knew that this doc sprint was coming up so I havn't updated any docs on doc.moodle.org.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Raj, don't forget the lang_string class that has been introduced in Moodle 2.3 No documentation at all for that, I did document the code but I knew that this doc sprint was coming up so I havn't updated any docs on doc.moodle.org. Cheers Sam
          Hide
          Rajesh Taneja added a comment - - edited

          Thanks for heads-up Sam
          Will try update this as well.

          Show
          Rajesh Taneja added a comment - - edited Thanks for heads-up Sam Will try update this as well.
          Hide
          Martin Dougiamas added a comment -

          I put some hints on http://docs.moodle.org/dev/String_API for you. I know the name isn't perfect but that's what we have always referred to it is. I think changing the name to I18N API or something would just be confusing for many developers.

          Show
          Martin Dougiamas added a comment - I put some hints on http://docs.moodle.org/dev/String_API for you. I know the name isn't perfect but that's what we have always referred to it is. I think changing the name to I18N API or something would just be confusing for many developers.
          Hide
          Rajesh Taneja added a comment -

          Thanks Martin
          Updated doc page

          Show
          Rajesh Taneja added a comment - Thanks Martin Updated doc page
          Hide
          Rajesh Taneja added a comment -

          FYI:
          string belongs to core package (which is defined in file @package tag), hence no package tag is added.

          Show
          Rajesh Taneja added a comment - FYI: string belongs to core package (which is defined in file @package tag), hence no package tag is added.
          Hide
          Sam Hemelryk added a comment -

          Hi Raj,

          Been looking at this one now and made the following notes:

          • Should get_parent_language be included in the string API (perhaps just my misunderstanding I'd always thought of us having an language API)
          • Same with moodle_setlocale, setup_lang_from_browser
          • What about shorten_text which is more on the string side of the bridge?
          • Should core_string_manager, string_manager et al have @category string?
          • translation_exists (on multiple classes) @param for $includeall needs to be fixed
          • @access private on __toString and __set_state + friends is wrong. You can call those publicly if you want.

          Otherwise spot on thanks dude.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Raj, Been looking at this one now and made the following notes: Should get_parent_language be included in the string API (perhaps just my misunderstanding I'd always thought of us having an language API) Same with moodle_setlocale, setup_lang_from_browser What about shorten_text which is more on the string side of the bridge? Should core_string_manager, string_manager et al have @category string? translation_exists (on multiple classes) @param for $includeall needs to be fixed @access private on __toString and __set_state + friends is wrong. You can call those publicly if you want. Otherwise spot on thanks dude. Cheers Sam
          Hide
          Rajesh Taneja added a comment -

          Thanks for the feedback Sam,

          1. Was not sure of get_parent_language, current_language, locale etc. I think it should be visible to plugin writer, in addition to it substr, s and p should also be visible as part of string api.
          2. Adding core_string_manager also make sense.
          3. Will fix @access as well.
          Show
          Rajesh Taneja added a comment - Thanks for the feedback Sam, Was not sure of get_parent_language, current_language, locale etc. I think it should be visible to plugin writer, in addition to it substr, s and p should also be visible as part of string api. Adding core_string_manager also make sense. Will fix @access as well.
          Hide
          Rajesh Taneja added a comment -

          Updated branch and doc.

          Show
          Rajesh Taneja added a comment - Updated branch and doc.
          Hide
          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
          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
          Rajesh Taneja added a comment -

          Branch re-based.

          Show
          Rajesh Taneja added a comment - Branch re-based.
          Hide
          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
          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
          Rajesh Taneja added a comment -

          Branch re-based.

          Show
          Rajesh Taneja added a comment - Branch re-based.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Everything looks perfect, but are you 100% sure we want to put under the same umbrella (string), both the i18n (translation) support and the strings handling facilities.

          That decision sounds really "forced" here. Perhaps we should split them?

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Everything looks perfect, but are you 100% sure we want to put under the same umbrella (string), both the i18n (translation) support and the strings handling facilities. That decision sounds really "forced" here. Perhaps we should split them? Ciao
          Hide
          Rajesh Taneja added a comment -

          Hello Eloy,

          AFAIK i18n and string should go under one category.
          http://docs.moodle.org/dev/index.php?title=String_API&action=historysubmit&diff=31440&oldid=31214

          Show
          Rajesh Taneja added a comment - Hello Eloy, AFAIK i18n and string should go under one category. http://docs.moodle.org/dev/index.php?title=String_API&action=historysubmit&diff=31440&oldid=31214
          Hide
          Eloy Lafuente (stronk7) added a comment -

          AFAIK i18n and string should go under one category.
          http://docs.moodle.org/dev/index.php?title=String_API&action=historysubmit&diff=31440&oldid=31214

          There is some people that should be prevented to write in some dev pages, LOL !

          IMO it's sooo clear that they are different beasts... anyway, np, if it's decided it's decided, split can always happen later... so integrating.

          Thanks!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - AFAIK i18n and string should go under one category. http://docs.moodle.org/dev/index.php?title=String_API&action=historysubmit&diff=31440&oldid=31214 There is some people that should be prevented to write in some dev pages, LOL ! IMO it's sooo clear that they are different beasts... anyway, np, if it's decided it's decided, split can always happen later... so integrating. Thanks! Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

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

          nobody tested this.

          Show
          Eloy Lafuente (stronk7) added a comment - nobody tested this.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          A bit later this week, but finally your changes have been accepted and are now available in all the upstream git/cvs servers.

          Many thanks & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - A bit later this week, but finally your changes have been accepted and are now available in all the upstream git/cvs servers. Many thanks & ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: