> On Dec. 14, 2013, 7:39 p.m., Paul Belanger wrote: > > /branches/12/rest-api/api-docs/bridges.json, lines 505-514 > > <https://reviewboard.asterisk.org/r/3070/diff/3/?file=49607#file49607line505> > > > > Why are these required, but every place else appear to be optional? > > Jonathan Rose wrote: > They are guaranteed to at least appear in the output even if they are > blank. I don't really know how appropriate that is. It might be better to > have them be '<none>' like with the manager events... I'm not sure, so I'll > wait for dlee or kmoore's opinion on this one as well.
If the parameter is not used, it should have a return value as Null, not a string with the value of '<none>'. This will be a nightmare to parse. > On Dec. 14, 2013, 7:39 p.m., Paul Belanger wrote: > > /branches/12/res/ari/ari_model_validators.h, lines 1066-1067 > > <https://reviewboard.asterisk.org/r/3070/diff/3/?file=49603#file49603line1066> > > > > any change on dropping the bridge_ prefix. Seems redundant this is a > > bridge model. > > > > For example, we have id not bridge_id, same with channel, not > > bridge_channels. > > > > Sadly, bridge_type, and bridge_class existing already. > > > > -1 for inconsistent variable names > > Jonathan Rose wrote: > I feel that this fits in better with bridge_type and bridge_class > conceptually. With channels being named as channels instead of > bridge_channels, it makes sense to deviate from that because channels are > another resource. And id... well, that's just always ID. > > I don't know if I'd really describe it as being inconsistent. I'll let > dlee or kmoore chime in if they are able though. Defiantly inconsistent, just look at the channel model. It is not channel_name, it is name. Same for CallerID, it is not callerid_name. Events is the red headed step child, it is eventname. So you can see, if we move forward with this, we'll have 3 different way to parse the name variable in our models. Additionally, we have changed the format of the variables again. We seem to be back to snake_case, I believe the decision was to keep everything camelCase[1]. [1] http://lists.digium.com/pipermail/asterisk-dev/2013-October/063003.html - Paul ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3070/#review10418 ----------------------------------------------------------- On Dec. 13, 2013, 9:40 p.m., Jonathan Rose wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3070/ > ----------------------------------------------------------- > > (Updated Dec. 13, 2013, 9:40 p.m.) > > > Review request for Asterisk Developers, David Lee, kmoore, Mark Michelson, > and rmudgett. > > > Repository: Asterisk > > > Description > ------- > > So there has been at least a little bit of clammoring internally about having > the need to be able to tell when a bridge belongs to a specific parking lot > or app_confbridge conference. This patch provides two new properties to the > bridge. > > BridgeCreator - Provides the name of a system which is responsible for > creating the bridge. This includes things such as 'AgentPool', 'Parking', > 'ConfBridge', 'Stasis', and other systems which can create bridges. > > BridgeName - Provides the name given to the bridge to refer to it internally > by the creator. > > BridgeCreator may be set or it may be unset (in which case it will appear as > a zero length string). BridgeName will only appear when BridgeCreator is > also set, but it is also optional. So you have the following possibilities: > > neither BridgeCreator nor BridgeName > BridgeCreator but not BridgeName > BridgeCreator and BridgeName > > > Diffs > ----- > > /branches/12/rest-api/api-docs/bridges.json 403725 > /branches/12/res/res_stasis.c 403725 > /branches/12/res/parking/parking_bridge.c 403725 > /branches/12/res/ari/ari_model_validators.c 403725 > /branches/12/res/ari/ari_model_validators.h 403725 > /branches/12/main/stasis_bridges.c 403725 > /branches/12/main/manager_bridges.c 403725 > /branches/12/main/bridge_basic.c 403725 > /branches/12/main/bridge.c 403725 > /branches/12/include/asterisk/stasis_bridges.h 403725 > /branches/12/include/asterisk/bridge_internal.h 403725 > /branches/12/include/asterisk/bridge.h 403725 > /branches/12/doc/appdocsxml.xslt 403725 > /branches/12/apps/app_confbridge.c 403725 > /branches/12/apps/app_bridgewait.c 403725 > /branches/12/apps/app_agent_pool.c 403725 > > Diff: https://reviewboard.asterisk.org/r/3070/diff/ > > > Testing > ------- > > Checked output for Manager events displaying Bridge snapshots > > Event: BridgeEnter > Privilege: call,all > BridgeUniqueid: a72735f6-04ac-499b-a2da-bb997d6f99ab > BridgeType: parking > BridgeTechnology: holding_bridge > BridgeCreator: Parking > BridgeName: default > BridgeNumChannels: 1 > Channel: PJSIP/pjgold-00000000 > ChannelState: 6 > ChannelStateDesc: Up > CallerIDNum: pjgold > CallerIDName: pjgold > ConnectedLineNum: <unknown> > ConnectedLineName: <unknown> > AccountCode: > Context: default > Exten: 700 > Priority: 1 > Uniqueid: 1386949677.0 > > > Checked output of GET /bridges on ARI in petstore: > > [ > { > "id": "a72735f6-04ac-499b-a2da-bb997d6f99ab", > "channels": [], > "technology": "holding_bridge", > "bridge_creator": "Parking", > "bridge_class": "parking", > "bridge_type": "holding", > "bridge_name": "default" > }, > { > "id": "80e19b8f-5a04-464a-885c-4c119764bf83", > "channels": [ > "1386949828.4" > ], > "technology": "holding_bridge", > "bridge_creator": "BridgeWait", > "bridge_class": "base", > "bridge_type": "holding", > "bridge_name": "testname" > } > ] > > > Thanks, > > Jonathan Rose > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
