Moodle
  1. Moodle
  2. MDL-30994

webservice API, check and update DocBlock

    Details

    • Rank:
      37398

      Description

      Check and update documentation, so that it should comply with moodle coding guidelines.
      Following needs to be updated/checked for webservice 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
          Michael de Raadt added a comment - - edited

          Some comments...

          webservice/amf/introspector.php

          • If, the description starting at line 43 is not a single-line comment block, it should be a proper multi-line block with the description starting on the next line.
          • Line 112, "methods description" should be "method descriptions"
          • Line 122, "classes descriptions" should be "class descriptions"

          webservice/amf/locallib.php

          • There should be blank comment line after the description at line 41

          webservice/externallib.php

          • Line 61, should "we return just known information by logged user" be "we return only known information about logged user"?
          • For the deprecated classes, it would be good to see a @todo to the MDL to remove them

          webservice/lib.php

          • Line 228, "a authorised user with his options" should be "an authorised user with their options"
          • Lines 251 and 308 "of a specific user" should be "for a specific user"
          • Line 381, "It also delete the function" should be "Also delete function"
          • Line 510, "usefull" should be "useful"
          • Line 529, "Get users missing" should be "Get missing user"
          • Lines 785 and 965 "Contructor" should be "Constructor"
          • Line 947, "allow" should be "allows"
          • Line 1239, "to know what is the type of the parameters" should be "to know the parameter types"

          webservice/renderer.php

          • Line 99, "authorised user" should be "authorised users"
          • Line 193, "Display list of function" should be "Display a list of functions"
          • Line 194, "build-in do" should be "built-in, do"
          • Line 448 should be "It is indented to be output within <pre> tags"
          • Lines 451 and 501, "HTML code. it is composed by space only." should be "HTML code; should contain only spaces."
          • Line 550, "colored" should be "coloured"
          • Line 621, "This display" should be "Displays"

          webservice/simpletest/testwebservice.php

          • Line 34, "this unit tests" should be "these unit tests"
          • Lines 35 and 36, "in the Moodle administration" should be "using Moodle site administration"
          • Line 37 doesn't make sense, should it be "Create a token and associate this service to an admin"?
          • Line 42, "number of time" should be "number of times"
          • Line 43, "run WRITE" should be "run the WRITE"
          • Line 49, "name by 'test'" should be "name with 'test'"
          • Line 57, "during the tests" should be "during tests"
          • Line 60, 63 and 66, "with X protocol" should be "with the X protocol"
          • Line 78, "the read only functions you want to test" should be "to test only read functions"
          • Line 81, "the read only functions you want to test" should be "to test only write functions"
          • What would happen if I set both of these to true?

          webservice/soap/locallib.php

          • Line 42, "are reverse to those of SoapFault." should be "are the reverse of those used by SoapFault"
          • Line 43, "integrated to the" should be "integrated in the"
          • Line 156, "formatted as XML" should be "formatted as an XML"
          • There should be a blank comment line after the description at line 230.

          That was a hefty piece of work. Well done.

          Show
          Michael de Raadt added a comment - - edited Some comments... webservice/amf/introspector.php If, the description starting at line 43 is not a single-line comment block, it should be a proper multi-line block with the description starting on the next line. Line 112, "methods description" should be "method descriptions" Line 122, "classes descriptions" should be "class descriptions" webservice/amf/locallib.php There should be blank comment line after the description at line 41 webservice/externallib.php Line 61, should "we return just known information by logged user" be "we return only known information about logged user"? For the deprecated classes, it would be good to see a @todo to the MDL to remove them webservice/lib.php Line 228, "a authorised user with his options" should be "an authorised user with their options" Lines 251 and 308 "of a specific user" should be "for a specific user" Line 381, "It also delete the function" should be "Also delete function" Line 510, "usefull" should be "useful" Line 529, "Get users missing" should be "Get missing user" Lines 785 and 965 "Contructor" should be "Constructor" Line 947, "allow" should be "allows" Line 1239, "to know what is the type of the parameters" should be "to know the parameter types" webservice/renderer.php Line 99, "authorised user" should be "authorised users" Line 193, "Display list of function" should be "Display a list of functions" Line 194, "build-in do" should be "built-in, do" Line 448 should be "It is indented to be output within <pre> tags" Lines 451 and 501, "HTML code. it is composed by space only." should be "HTML code; should contain only spaces." Line 550, "colored" should be "coloured" Line 621, "This display" should be "Displays" webservice/simpletest/testwebservice.php Line 34, "this unit tests" should be "these unit tests" Lines 35 and 36, "in the Moodle administration" should be "using Moodle site administration" Line 37 doesn't make sense, should it be "Create a token and associate this service to an admin"? Line 42, "number of time" should be "number of times" Line 43, "run WRITE" should be "run the WRITE" Line 49, "name by 'test'" should be "name with 'test'" Line 57, "during the tests" should be "during tests" Line 60, 63 and 66, "with X protocol" should be "with the X protocol" Line 78, "the read only functions you want to test" should be "to test only read functions" Line 81, "the read only functions you want to test" should be "to test only write functions" What would happen if I set both of these to true? webservice/soap/locallib.php Line 42, "are reverse to those of SoapFault." should be "are the reverse of those used by SoapFault" Line 43, "integrated to the" should be "integrated in the" Line 156, "formatted as XML" should be "formatted as an XML" There should be a blank comment line after the description at line 230. That was a hefty piece of work. Well done.
          Hide
          Jérôme Mouneyrac added a comment -

          No worries Mike, I'll fix that today Thanks for the peer-review too, it is quite long.

          Show
          Jérôme Mouneyrac added a comment - No worries Mike, I'll fix that today Thanks for the peer-review too, it is quite long.
          Hide
          Jérôme Mouneyrac added a comment -

          I did the changes, thanks for all the catches Mike. Submitted for integration.

          Show
          Jérôme Mouneyrac added a comment - I did the changes, thanks for all the catches Mike. Submitted for integration.
          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 -
          • webservice/lib.php (#260, incorrectly aligned // comments).
          • webservice/lib.php (#640, strange alignment)
          • webservice/simpletest/testwebservice.php (no scope for "moodle_notes_create_notes")
          Show
          Eloy Lafuente (stronk7) added a comment - webservice/lib.php (#260, incorrectly aligned // comments). webservice/lib.php (#640, strange alignment) webservice/simpletest/testwebservice.php (no scope for "moodle_notes_create_notes")
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Ping!

          Show
          Eloy Lafuente (stronk7) added a comment - Ping!
          Hide
          Jérôme Mouneyrac added a comment -

          sorry just saw it yesterday, fixing it...

          Show
          Jérôme Mouneyrac added a comment - sorry just saw it yesterday, fixing it...
          Hide
          Jérôme Mouneyrac added a comment -

          I didn't get the comment:

          • webservice/simpletest/testwebservice.php (no scope for "moodle_notes_create_notes")
            What does that mean? This function looks similar to others...

          submitting let me know if anythign wrong

          Show
          Jérôme Mouneyrac added a comment - I didn't get the comment: webservice/simpletest/testwebservice.php (no scope for "moodle_notes_create_notes") What does that mean? This function looks similar to others... submitting let me know if anythign wrong
          Hide
          Eloy Lafuente (stronk7) added a comment -
          • Error: The MDL-30994 branch at git://github.com/mouneyrac/moodle.git does not apply clean to master. (conflict @ webservice/rest/lib.php).
          • About the 'no scope for "moodle_notes_create_notes"' error, it's missing any public/protected/private specification. It's true that other methods in the class are also missing it but, as far as the checker only looks for modified lines... only that was detected. I think it would be great to add all the missing scopes (all public if I'm not wrong).

          Reopening...

          Show
          Eloy Lafuente (stronk7) added a comment - Error: The MDL-30994 branch at git://github.com/mouneyrac/moodle.git does not apply clean to master. (conflict @ webservice/rest/lib.php). About the 'no scope for "moodle_notes_create_notes"' error, it's missing any public/protected/private specification. It's true that other methods in the class are also missing it but, as far as the checker only looks for modified lines... only that was detected. I think it would be great to add all the missing scopes (all public if I'm not wrong). Reopening...
          Hide
          Jérôme Mouneyrac added a comment -

          I rebased and fixed the issues. The phpdoc checker returns no errors.
          I created an issue for the unit test missing function scopes MDL-31710
          Cheers

          Show
          Jérôme Mouneyrac added a comment - I rebased and fixed the issues. The phpdoc checker returns no errors. I created an issue for the unit test missing function scopes MDL-31710 Cheers
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Sorry, I'm reopening this because it is also conflicting and, while the collision is only one, I'm not really sure which version is better.

          Your last change talks about $_REQUEST, but the previous change talks about $_POST and $_GET instead, that seems more accurate looking to the code.

          So, for your consideration. Easy to fix. Remember you'll need to rebase against tomorrow's weeklies, to be sure you're putting your commits on latest top.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Sorry, I'm reopening this because it is also conflicting and, while the collision is only one, I'm not really sure which version is better. Your last change talks about $_REQUEST, but the previous change talks about $_POST and $_GET instead, that seems more accurate looking to the code. So, for your consideration. Easy to fix. Remember you'll need to rebase against tomorrow's weeklies, to be sure you're putting your commits on latest top. Ciao
          Hide
          Jérôme Mouneyrac added a comment -

          yes we need to keep the recent $_POST and $_GET phpdoc change. I'll fix this new conflict after weeklies, hopefully we'll don't have another new conflict.

          Show
          Jérôme Mouneyrac added a comment - yes we need to keep the recent $_POST and $_GET phpdoc change. I'll fix this new conflict after weeklies, hopefully we'll don't have another new conflict.
          Hide
          Jérôme Mouneyrac added a comment -

          Done Eloy, rebased, fixed the new conflict and repushed. Can you integrate this before anything else? Thanks a lot

          Show
          Jérôme Mouneyrac added a comment - Done Eloy, rebased, fixed the new conflict and repushed. Can you integrate this before anything else? Thanks a lot
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Thanks for the suggestion. I'll try to integrate it the last.

          (LOL, sorry, couldn't resist). Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Thanks for the suggestion. I'll try to integrate it the last. (LOL, sorry, couldn't resist). Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Jérôme Mouneyrac added a comment -

          ahah

          Show
          Jérôme Mouneyrac added a comment - ahah
          Hide
          Michael de Raadt added a comment -

          I proofed the related API doc.

          Show
          Michael de Raadt added a comment - I proofed the related API doc.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!).

          icao_reverse('arreis olik rebemevon afla letoh ognat');
          

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!). icao_reverse('arreis olik rebemevon afla letoh ognat'); Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: