Details

      Description

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

          Issue Links

            Activity

            Hide
            Rajesh Taneja added a comment -

            Still working on it, adding Pull branches for reference.

            Show
            Rajesh Taneja added a comment - Still working on it, adding Pull branches for reference.
            Hide
            Rajesh Taneja added a comment -

            will update 2.2 after peer review.

            Show
            Rajesh Taneja added a comment - will update 2.2 after peer review.
            Hide
            Martin Dougiamas added a comment -

            This is the docs page: http://docs.moodle.org/dev/Form_API

            Show
            Martin Dougiamas added a comment - This is the docs page: http://docs.moodle.org/dev/Form_API
            Hide
            Rajesh Taneja added a comment -

            Can you please provide feedback, before I create 2.2 branch and push it for integration.

            Show
            Rajesh Taneja added a comment - Can you please provide feedback, before I create 2.2 branch and push it for integration.
            Hide
            Rajesh Taneja added a comment -

            Updated form API doc page

            Show
            Rajesh Taneja added a comment - Updated form API doc page
            Hide
            Sam Hemelryk added a comment - - edited

            Hi Raj,

            I've been looking at this just now, the following are my notes:

            1. The @abstract tag is only valid in PHP 4, PHP 5 has a keyword abstract. (pasted from the phpdoc manual) No need to specify that any more as we require php5 of course exception to that is if we have old code with abstract classes that are presumed abstract and not actually noted as abstract by the keyword.
            2. Several boolean and integer's around the place.
            3. formslib.php
              • moodleform class
                • $customdata doesn't have to be an array it can be anything
                • Please fix the alignment of multiline params in moodleform method
                • save_files method needs @deprecated tag
                • get_new_filename comma after $elname should be removed
                • save_stored_file missing an @param
              • MoodleQuickForm class
                • MoodleQuickForm method $action is either a string of a moodle_url
                • accept $renderer appears to be a HTML_QuickForm_Renderer object
                • setType includes a limited set of examples for PARAM_WHATEVER uses. Not your doing just noting that is totally in the wrong place. If we do provide an example it should be a method example not a param example. Actually PARAM_ACTION is deprecated and a bad example anyway lol.. would you mind tiding that up somehow Raj?
                • exportValues missing @return
            4. Several elements have a typo in the setHelpButton method: $help => $helpbuttonargs and also missin deprecated tags etc.
            5. Missing docblock for MoodleQuickForm_checkbox constructor
            6. checkbox also missing deprecated etc from setHelpButton
            7. Please tidy up alignment on MoodleQuickForm_date_selector line 44.. that is a whole new way of doing a docblock and we need to minimise the number of different ways we do things. (in a couple of other places as well wont be mentioned again)
            8. date_selector accept $renderer is a HTML_QuickForm_Renderer (same in other places won't be mentioned again)
            9. form_filemanaer_x missing phpdoc block for property and constructor
            10. MoodleQuickForm_htmleditor missing php doc blocks for properties and constructor
            11. selectwithlink::removeOptions incorrect @param

            On an interesting note I noticed several docblocks like the following:

            /** @var string String with the html for hidden params passed in as part of a moodle_url
             * object for the action. Output in the form. */
            

            Do you know whether that sort of docblock validates OK? (I've never seen it before these changes)
            Actually you introduced a couple of different variants to this.
            I think we need to really limit the number of different ways we create phpdoc blocks.
            I think the following two should be acceptable and the rest be changed:

            /** A single line comment **/
             
            /**
             * A multiline or single comment
             * should look like this
             */
            

            Again all about consistency and example. Perhaps I'm being to up tight about it but the more variations I see the less I like our documentation.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - - edited Hi Raj, I've been looking at this just now, the following are my notes: The @abstract tag is only valid in PHP 4, PHP 5 has a keyword abstract. (pasted from the phpdoc manual) No need to specify that any more as we require php5 of course exception to that is if we have old code with abstract classes that are presumed abstract and not actually noted as abstract by the keyword. Several boolean and integer's around the place. formslib.php moodleform class $customdata doesn't have to be an array it can be anything Please fix the alignment of multiline params in moodleform method save_files method needs @deprecated tag get_new_filename comma after $elname should be removed save_stored_file missing an @param MoodleQuickForm class MoodleQuickForm method $action is either a string of a moodle_url accept $renderer appears to be a HTML_QuickForm_Renderer object setType includes a limited set of examples for PARAM_WHATEVER uses. Not your doing just noting that is totally in the wrong place. If we do provide an example it should be a method example not a param example. Actually PARAM_ACTION is deprecated and a bad example anyway lol.. would you mind tiding that up somehow Raj? exportValues missing @return Several elements have a typo in the setHelpButton method: $help => $helpbuttonargs and also missin deprecated tags etc. Missing docblock for MoodleQuickForm_checkbox constructor checkbox also missing deprecated etc from setHelpButton Please tidy up alignment on MoodleQuickForm_date_selector line 44.. that is a whole new way of doing a docblock and we need to minimise the number of different ways we do things. (in a couple of other places as well wont be mentioned again) date_selector accept $renderer is a HTML_QuickForm_Renderer (same in other places won't be mentioned again) form_filemanaer_x missing phpdoc block for property and constructor MoodleQuickForm_htmleditor missing php doc blocks for properties and constructor selectwithlink::removeOptions incorrect @param On an interesting note I noticed several docblocks like the following: /** @var string String with the html for hidden params passed in as part of a moodle_url * object for the action. Output in the form. */ Do you know whether that sort of docblock validates OK? (I've never seen it before these changes) Actually you introduced a couple of different variants to this. I think we need to really limit the number of different ways we create phpdoc blocks. I think the following two should be acceptable and the rest be changed: /** A single line comment **/   /** * A multiline or single comment * should look like this */ Again all about consistency and example. Perhaps I'm being to up tight about it but the more variations I see the less I like our documentation. Cheers Sam
            Hide
            Sam Hemelryk added a comment -

            BTW all in all very good dude, most of the noted things are just very minor and are easily missed.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - BTW all in all very good dude, most of the noted things are just very minor and are easily missed. Cheers Sam
            Hide
            Rajesh Taneja added a comment -

            Thanks Sam,
            Branch updated with all the changes (you suggested and few more.)
            As discussed, pushing it up for integration review.

            Show
            Rajesh Taneja added a comment - Thanks Sam, Branch updated with all the changes (you suggested and few more.) As discussed, pushing it up for integration review.
            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 -

            missing repository url

            Show
            Eloy Lafuente (stronk7) added a comment - missing repository url
            Hide
            Rajesh Taneja added a comment -

            Sorry Eloy
            Added repository URL.

            Show
            Rajesh Taneja added a comment - Sorry Eloy Added repository URL.
            Hide
            Sam Hemelryk added a comment -

            Thanks Raj, this has been integrated now

            Show
            Sam Hemelryk added a comment - Thanks Raj, this has been integrated now
            Hide
            Adrian Greeve added a comment -

            This all looks good. Thanks.

            Show
            Adrian Greeve added a comment - This all looks good. Thanks.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Some changes to Moodle should be milestones in the project by themselves.

            This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks!

            Closing as fixed, ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Some changes to Moodle should be milestones in the project by themselves. This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks! Closing as fixed, ciao
            Hide
            Aparup Banerjee added a comment -

            added MDL-31294 which is removing deprecated setHelpButton()

            also handled there are :

            • removing MoodleQuickForm_file
            • removing MoodleQuickForm_format
            Show
            Aparup Banerjee added a comment - added MDL-31294 which is removing deprecated setHelpButton() also handled there are : removing MoodleQuickForm_file removing MoodleQuickForm_format

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: