[Asterisk-code-review] loader: Add dependency fields to module structures. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/7872 )

Change subject: loader: Add dependency fields to module structures.
......................................................................


Patch Set 5: Code-Review-1

(6 comments)

https://gerrit.asterisk.org/#/c/7872/5/include/asterisk/module.h
File include/asterisk/module.h:

https://gerrit.asterisk.org/#/c/7872/5/include/asterisk/module.h@342
PS5, Line 342: 	/*! Modules which must be started first, in comma-separated string format. */
             : 	const char *requires;
This seems to need more clarification.

Does this mean the listed modules must ALWAYS be loaded and started for this module to function?

Or does it mean that if any of the listed modules are loaded they need to be started before this module?  (Which sounds more like optional_apis.)


https://gerrit.asterisk.org/#/c/7872/5/res/res_agi.c
File res/res_agi.c:

https://gerrit.asterisk.org/#/c/7872/5/res/res_agi.c@4720
PS5, Line 4720: 	.requires = "res_speech",
Why is this required for AGI?  It should be optional.


https://gerrit.asterisk.org/#/c/7872/5/res/res_ari.c
File res/res_ari.c:

https://gerrit.asterisk.org/#/c/7872/5/res/res_ari.c@1200
PS5, Line 1200: 	.optional_apis = "res_http_websocket",
Heh.  I thought ARI was essentially nonfunctional without a websocket.


https://gerrit.asterisk.org/#/c/7872/5/res/res_pjsip.c
File res/res_pjsip.c:

https://gerrit.asterisk.org/#/c/7872/5/res/res_pjsip.c@5128
PS5, Line 5128: 	.requires = "res_pjproject,res_pjsip_config_wizard",
Requires config wizard?  I thought that was optional.


https://gerrit.asterisk.org/#/c/7872/5/res/res_pjsip_outbound_publish.c
File res/res_pjsip_outbound_publish.c:

https://gerrit.asterisk.org/#/c/7872/5/res/res_pjsip_outbound_publish.c@1705
PS5, Line 1705: 	.requires = "res_pjproject,res_pjsip",
Should only need res_pjsip as res_pjsip cannot function without res_pjproject.


https://gerrit.asterisk.org/#/c/7872/5/res/res_pjsip_registrar.c
File res/res_pjsip_registrar.c:

https://gerrit.asterisk.org/#/c/7872/5/res/res_pjsip_registrar.c@1137
PS5, Line 1137: 	.requires = "res_pjproject,res_pjsip",
Should only need res_pjsip as res_pjsip cannot function without res_pjproject.



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ad9547a0a6442409ff4e352a6d897bef2cc04bf
Gerrit-Change-Number: 7872
Gerrit-PatchSet: 5
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Sun, 14 Jan 2018 22:16:19 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180114/73ddfbd4/attachment.html>