Moodle
  1. Moodle
  2. MDL-30983

calendar API, phpdocs and devdocs

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.2.1
    • Fix Version/s: 2.3
    • Component/s: Calendar, Documentation
    • Labels:
    • Rank:
      37387

      Description

      Check and update documentation, so that it should comply with moodle coding guidelines.
      Following needs to be updated/checked for calendar 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.

        Issue Links

          Activity

          Hide
          David Mudrak added a comment -

          Hi Rosi, please note doc block templates are disallowed.

          Show
          David Mudrak added a comment - Hi Rosi, please note doc block templates are disallowed.
          Hide
          Rajesh Taneja added a comment - - edited

          Hello Rossie,

          In addition to David's comment, you might want to consider following:

          1. Don't need to define package and category for every function, just add it for class and functions in file (outside class) which you want to show to plugin developer. Package is not required on functions as it will be picked by page level docblock.
          2. Most of the params doesn't have description
          3. Please don't do inline changes "calendar/lib.php - line 172", it s all over in patch. Just indent/add/update docblock (outside function)
          4. When using @deprecated, mention version of moodle and add @see to refer to new function (if available)

          Rest looks Good

          Show
          Rajesh Taneja added a comment - - edited Hello Rossie, In addition to David's comment, you might want to consider following: Don't need to define package and category for every function, just add it for class and functions in file (outside class) which you want to show to plugin developer. Package is not required on functions as it will be picked by page level docblock. Most of the params doesn't have description Please don't do inline changes "calendar/lib.php - line 172", it s all over in patch. Just indent/add/update docblock (outside function) When using @deprecated, mention version of moodle and add @see to refer to new function (if available) Rest looks Good
          Hide
          Rossiani Wijaya added a comment -

          Hi Raj,

          1. The first half of the file is just a regular function. So it only needs @category?
          2. will update this
          3. for docblock, do we fix the format? for example (notice the empty space infront of the second line) :

          /**
          * something something something
          * something something something
          */

          To

          /**
           * something something something
           * something something something
           */

          4. will update this

          Show
          Rossiani Wijaya added a comment - Hi Raj, 1. The first half of the file is just a regular function. So it only needs @category? 2. will update this 3. for docblock, do we fix the format? for example (notice the empty space infront of the second line) : /** * something something something * something something something */ To /** * something something something * something something something */ 4. will update this
          Hide
          Martin Dougiamas added a comment -

          Rossi,

          The only part of the calendar files that needs the "@category calendar" tag is the class calendar_event, as that is the public-facing API. (Unless I'm mistaken)

          Likewise this is all that needs to be discussed on http://docs.moodle.org/dev/Calendar_API

          Show
          Martin Dougiamas added a comment - Rossi, The only part of the calendar files that needs the "@category calendar" tag is the class calendar_event, as that is the public-facing API. (Unless I'm mistaken) Likewise this is all that needs to be discussed on http://docs.moodle.org/dev/Calendar_API
          Hide
          Martin Dougiamas added a comment -

          All looks OK to me (phpdocs etc) except I'm not seeing "@category calendar" tag on the class calendar_event.

          Can you work on http://docs.moodle.org/dev/Calendar_API ?

          Show
          Martin Dougiamas added a comment - All looks OK to me (phpdocs etc) except I'm not seeing "@category calendar" tag on the class calendar_event. Can you work on http://docs.moodle.org/dev/Calendar_API ?
          Hide
          Rossiani Wijaya added a comment - - edited

          Thanks Martin for reviewing.

          I updated the patch to add @category for classes.

          Working on Calendar_API (dev doc) now.

          Show
          Rossiani Wijaya added a comment - - edited Thanks Martin for reviewing. I updated the patch to add @category for classes. Working on Calendar_API (dev doc) now.
          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
          Eloy Lafuente (stronk7) added a comment -

          Both the phpdocs and the moodledocs looks good enough. The only point I was not able to understand completely (I didn't know it existed in fact) was how the hook is expected to work, and the purpose (contents) of that $CFG->calendar setting. Perhaps some example with a hook skeleton (using switch() for actions...) could be interesting?

          Good work! Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Both the phpdocs and the moodledocs looks good enough. The only point I was not able to understand completely (I didn't know it existed in fact) was how the hook is expected to work, and the purpose (contents) of that $CFG->calendar setting. Perhaps some example with a hook skeleton (using switch() for actions...) could be interesting? Good work! 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, passing.

          Show
          Eloy Lafuente (stronk7) added a comment - nobody tested this, passing.
          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:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: