[Asterisk-code-review] ARI: Added new functionality to get all module information. (asterisk[master])

[Benjamin Keith Ford]

Benjamin Keith Ford has posted comments on this change.

Change subject: ARI: Added new functionality to get all module information.
......................................................................


Patch Set 2:

(6 comments)

https://gerrit.asterisk.org/#/c/747/2/include/asterisk/module.h
File include/asterisk/module.h:

Line 162:                            const char *like);
> Since you added the \since tag on line 157, maybe fix this indentation whil
This is actually at the correct indention. It's not part of the callback, it's part of the ast_update_module_list parameters. It lines up with the starting parentheses from that function. My indentation on the new function is actually incorrect :)


Line 166:  * \param modentry A callback to an updated function
> Based on the documentation for ast_update_module_list, I think this is supp
Thanks for catching that!


Line 167:  * \param like
> What does this parameter do?
I'm unsure. It's not specified in the original function. Otherwise, I would have included the description.


Line 175: int ast_update_module_list_data(int (*modentry)(const char *module, const char *description,
        :                                                 int usecnt, const char *status, const char *like,
        :                                                 enum ast_module_support_level support_level,
        :                                                 void *data), const char *like, void *data);
> Does this function really update the module list data or does it just fetch
This function is essentially an exact replica of ast_update_module_list. This is how it was specified in my issue as well :)


https://gerrit.asterisk.org/#/c/747/2/res/ari/resource_asterisk.c
File res/ari/resource_asterisk.c:

Line 158: const char *like
> 'like' is not doing anything and is set as NULL when ast_update_module_list
Like may not be doing anything here, but if someone else wants to use it in one of their callbacks, it wouldn't be available to them if I removed it. Correct me if I'm wrong :)


Line 176: headers
> Is this parameter required as a parameter because of a contract? If it is n
It is required. This function is an exact copy of the signature from the header file, generated by make ari-stubs.


-- 
To view, visit https://gerrit.asterisk.org/747
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I63cbbf0ec0c3544cc45ed2a588dceabe91c5e0b0
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes