Details

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

      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.

        Activity

        Hide
        Aparup Banerjee added a comment -

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

        Show
        Aparup Banerjee added a comment - ok, this branch is up and ready for peer-review.
        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        Aparup Banerjee added a comment -

        updated branch with formatting changes to phpdocs.

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

        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
        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: