Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-30983

calendar API, phpdocs and devdocs

    Details

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

      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.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              mudrd8mz David Mudrák added a comment -

              Hi Rosi, please note doc block templates are disallowed.

              Show
              mudrd8mz David Mudrák added a comment - Hi Rosi, please note doc block templates are disallowed.
              Hide
              rajeshtaneja 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
              rajeshtaneja 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
              rwijaya 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
              rwijaya 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
              dougiamas 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
              dougiamas 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
              dougiamas 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
              dougiamas 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
              rwijaya 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
              rwijaya 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Integrated, thanks!

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

              nobody tested this, passing.

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - nobody tested this, passing.
              Hide
              stronk7 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
              stronk7 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:
                    Fix Release Date:
                    25/Jun/12