Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Documentation
    • Labels:

      Description

      check, update and tune the phpdocs and especially the API doc. Edit http://docs.moodle.org/dev/Data_definition_API such that it is aimed towards a plugin developer as the target audience.

        Gliffy Diagrams

          Activity

          Hide
          nebgor Aparup Banerjee added a comment -

          ok, this branch is up and ready for peer-review.

          Show
          nebgor Aparup Banerjee added a comment - ok, this branch is up and ready for peer-review.
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Aparup,
          I think you should run Marina's tool. It has few missing/incomplete doc blocks.

          In addition:

          1. @subpackage should be removed, as discussed it's not going to be used.
          2. @package should be core_ddl
          3. multiline define should be

            /** 
             * @var string Template to drop PKs.
             * 'TABLENAME' and 'KEYNAME' will be replaced from this template.
             */

            and not

            /** @var string Template to drop PKs.
             * 'TABLENAME' and 'KEYNAME' will be replaced from this template.
             */

          Rest looks Good.

          FYI:
          https://github.com/marinaglancy/moodle-local_moodlecheck

          Show
          rajeshtaneja Rajesh Taneja added a comment - Aparup, I think you should run Marina's tool. It has few missing/incomplete doc blocks. In addition: @subpackage should be removed, as discussed it's not going to be used. @package should be core_ddl multiline define should be /** * @var string Template to drop PKs. * 'TABLENAME' and 'KEYNAME' will be replaced from this template. */ and not /** @var string Template to drop PKs. * 'TABLENAME' and 'KEYNAME' will be replaced from this template. */ Rest looks Good. FYI: https://github.com/marinaglancy/moodle-local_moodlecheck
          Hide
          nebgor Aparup Banerjee added a comment - - edited

          Thanks Raj,
          Do you mean the enum functions are missing the phpdocs? they are deprecated as such to be removed (since 2.1 actually). Thus i left the phpdoc out as we really don't want them to be used.
          Actually, good that you mentioned that, i'll add the @todo to these deprecated functons referencing MDL-31147.

          1: http://docs.moodle.org/dev/Coding_style#.40package says @subpackage use is optional hence i used it to distinguish between ddl API and ddl implementations which aren't our API as they are instantiations of our abstract API.

          2: @package should be 'core' as 'ddl' atm isn't a subsystem in get_core_subsystems() as mentioned in http://docs.moodle.org/dev/Coding_style#.40package

          3. thanks for that, i'll clean that up

          ok thats all cleaned up and rebased. pushing up for integration now.

          Show
          nebgor Aparup Banerjee added a comment - - edited Thanks Raj, Do you mean the enum functions are missing the phpdocs? they are deprecated as such to be removed (since 2.1 actually). Thus i left the phpdoc out as we really don't want them to be used. Actually, good that you mentioned that, i'll add the @todo to these deprecated functons referencing MDL-31147 . 1: http://docs.moodle.org/dev/Coding_style#.40package says @subpackage use is optional hence i used it to distinguish between ddl API and ddl implementations which aren't our API as they are instantiations of our abstract API. 2: @package should be 'core' as 'ddl' atm isn't a subsystem in get_core_subsystems() as mentioned in http://docs.moodle.org/dev/Coding_style#.40package 3. thanks for that, i'll clean that up ok thats all cleaned up and rebased. pushing up for integration now.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          The only minor point I could argue is why in files like sql_generator.php, you are:

          1) repeating the @package / @subpackage / @category in the class if it's the same than the file one.
          2) why the abstract sql_generator has different subpackage (ddl) than its implementations (ddl_generator). I agree only the former must have the category (API), but also think that all them should have the same (optional) subpackage.

          Otherwise, it looks good, thanks!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - The only minor point I could argue is why in files like sql_generator.php, you are: 1) repeating the @package / @subpackage / @category in the class if it's the same than the file one. 2) why the abstract sql_generator has different subpackage (ddl) than its implementations (ddl_generator). I agree only the former must have the category (API), but also think that all them should have the same (optional) subpackage. Otherwise, it looks good, thanks!
          Hide
          nebgor Aparup Banerjee added a comment -

          Hi Eloy,
          1) whenever i saw the comments were differing ( and existing) between class docblock and the file's docblock i've in repeated the information so that it can be picked up. System X is supposed to be handling these apparently.

          2) sql_generator is the API and implementations aren't, at the moment the API has category and the rest don't but a person looking at the implementation file alone can't see that, thats why i've used @subpackage (optionally) to further clarify this (http://docs.moodle.org/dev/Coding_style#.40package) .

          Show
          nebgor Aparup Banerjee added a comment - Hi Eloy, 1) whenever i saw the comments were differing ( and existing) between class docblock and the file's docblock i've in repeated the information so that it can be picked up. System X is supposed to be handling these apparently. 2) sql_generator is the API and implementations aren't, at the moment the API has category and the rest don't but a person looking at the implementation file alone can't see that, thats why i've used @subpackage (optionally) to further clarify this ( http://docs.moodle.org/dev/Coding_style#.40package ) .
          Hide
          nebgor Aparup Banerjee added a comment -

          updated branch with formatting changes to phpdocs.

          Show
          nebgor Aparup Banerjee added a comment - updated branch with formatting changes to phpdocs.
          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

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

          It is late here and I'm very tired but I didn't want to go to sleep before expressing my admiration for your amazing collaboration. Thanks!

          Closing as fixed, heading to zzzZZZzzz, niao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - It is late here and I'm very tired but I didn't want to go to sleep before expressing my admiration for your amazing collaboration. Thanks! Closing as fixed, heading to zzzZZZzzz, niao

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                25/Jun/12