probably shouldn't really be
what we call the callback, except:
1) fixup is just as vague
2) I couldn't think of anything better.
Ship it!
- Mark Michelson
On July 28, 2014, 5:35 p.m., opticron wrote:
---
This is an automatically
---
I can confirm that this test does not pass with current trunk, but with the
patch applied, the test passes successfully.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com
by a
StasisEnd event. No further events are emitted for the channels over the
websocket.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE
that
the code only needs to be up for review to be a candidate for inclusion, I have
posted this without any testsuite tests. This will not be committed without
either modifying existing tests or adding new tests, though.
Thanks,
Mark Michelson
---
I have created both tracked and untracked contacts and ensured that the output
looked correct for each.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing
the string passed
in.
- Mark Michelson
On July 29, 2014, 2:40 p.m., Joshua Colp wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3780
On July 29, 2014, 2:32 p.m., Joshua Colp wrote:
/branches/1.8/main/manager.c, line 207
https://reviewboard.asterisk.org/r/3854/diff/1/?file=65275#file65275line207
This confuses me, even after reading a few times. Specifically what
you've added: directly or via function. Dialplan
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3799/#review12917
---
Ship it!
Ship it! (aside from opticron's findings)
- Mark
a ChannelLeftBridge, followed by a
StasisEnd event. No further events are emitted for the channels over the
websocket.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing
-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3857/#review12889
---
On July 28, 2014, 5:21 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail
Functions0
Running core
...
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit
,
Mark Michelson
--
_
-- 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
)
behave as expected. This lends some confidence to the idea that the front line
is working, at least.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
there is still no method of generating multipart/related or RLMI bodies.
However, with these changes, I did run the gamut of subscription tests in the
testsuite and they all pass. This at least means that there are no detectable
regressions at this point.
Thanks,
Mark Michelson
batched notification, and that operations that should cancel a batch
did so properly.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE
in these tests, so I figured I'd get those caught by
reviewers early.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit
On July 18, 2014, 3:40 p.m., Mark Michelson wrote:
/trunk/include/asterisk/res_pjsip.h, lines 1195-1203
https://reviewboard.asterisk.org/r/3817/diff/2/?file=64713#file64713line1195
This function isn't necessary. When PJSIP is passed a URI string, PJSIP
will perform URI validation
On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
/trunk/res/res_pjsip_outbound_publish.c, lines 71-75
https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line71
This seems like an odd default behavior. I would suspect that by
default we would actually magic up
On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
/trunk/res/res_pjsip_outbound_publish.c, lines 71-75
https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line71
This seems like an odd default behavior. I would suspect that by
default we would actually magic up
On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
/trunk/res/res_pjsip_outbound_publish.c, lines 71-75
https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line71
This seems like an odd default behavior. I would suspect that by
default we would actually magic up
should be good to go.
- Mark Michelson
On July 21, 2014, 8:54 a.m., Corey Farrell wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3759
On July 18, 2014, 5:18 p.m., opticron wrote:
I gave this review a look and only found the same findings as opticron.
- Mark
---
This is an automatically generated e-mail. To reply, visit:
time
ast_update_module_list() is called. Up to you really.
- Mark Michelson
On July 21, 2014, 4:58 p.m., Matt Jordan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3777
for ast_sockaddr_stringify() as before.
- Mark Michelson
On July 16, 2014, 10:37 p.m., Matt Jordan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3810
://reviewboard.asterisk.org/r/3781/#comment23110
There's an inline function in include/asterisk/netsock2.h called
ast_sockaddr_stringify_port() that you can use in place of the
ast_sockaddr_stringify_fmt() call.
- Mark Michelson
On July 21, 2014, 1:15 p.m., dtryba wrote
to this
function, then the NULL check in this function should be changed to an
assertion instead.
- Mark Michelson
On July 21, 2014, 5:37 p.m., opticron wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
://reviewboard.asterisk.org/r/3729/#comment23114
There's no real need for count when you have ao2_container_count()
available to you. You can cal it prior to iterating over the items since you're
removing the commands from the queue as they are executed.
- Mark Michelson
On July 18, 2014, 5:48 p.m., opticron
On July 17, 2014, 9:41 p.m., Matt Jordan wrote:
/trunk/main/stasis_channels.c, lines 1071-1077
https://reviewboard.asterisk.org/r/3811/diff/1/?file=64565#file64565line1071
I'm not sure why these changes (removal of the .to_ami callback) were
necessary.
Generally, I
if it cannot load the config file. If stasis_init() returns
non-zero, Asterisk will not start.
Have a look at main/udptl.c::__ast_udptl_reload() for a concise example of
how to set default configuration in the case of an error from the config
framework.
- Mark Michelson
On July 18
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3816/#review12730
---
On July 17, 2014, 6:01 p.m., Mark Michelson wrote
or URI is
acceptable.
- Mark Michelson
On July 17, 2014, 7:27 p.m., Jonathan Rose wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3817
.
However, given my comments from your Asterisk implementation review, the
pjsip.conf will need to be updated to use a default outbound endpoint.
- Mark Michelson
On July 17, 2014, 9:51 p.m., Jonathan Rose wrote
,
Mark Michelson
--
_
-- 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
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3815/#review12722
---
Ship it!
Ship It!
- Mark Michelson
On July 17, 2014, 5:57
DAHDI ISDN Remote Access Server 0
Running extended
app_db.so Database Access Functions0
Running core
...
Thanks,
Mark Michelson
this without any testsuite tests. This will not be committed without
either modifying existing tests or adding new tests, though.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com
-
/trunk/res/res_pjsip/pjsip_options.c 418633
Diff: https://reviewboard.asterisk.org/r/3797/diff/
Testing
---
I have created both tracked and untracked contacts and ensured that the output
looked correct for each.
Thanks,
Mark Michelson
app_dahdiras.soDAHDI ISDN Remote Access Server 0
Running extended
app_db.so Database Access Functions0
Running core
...
Thanks,
Mark Michelson
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3802/#review12674
---
On July 15, 2014, 10:35 p.m., Mark Michelson wrote
core
...
Thanks,
Mark Michelson
--
_
-- 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
looked correct in that case. I also tested batched notifications to ensure that
the notification was batched, that multiple state changes would be reflected in
a single batched notification, and that operations that should cancel a batch
did so properly.
Thanks,
Mark Michelson
On 07/09/2014 06:29 AM, Tzafrir Cohen wrote:
On Thu, Jul 03, 2014 at 12:02:35PM -, Diederik de Groot wrote:
Which should have been:
Cflags: -I/usr/include/libxml2 -g3
Off-topic: what is libxml2 needed for?
$ ls -F /usr/include/libxml2/
libxml/
$ grep -r libxml include/asterisk*
[
On 07/09/2014 05:30 PM, Matthew Jordan wrote:
On Wed, Jul 9, 2014 at 4:34 PM, bala murugan fightwit...@gmail.com wrote:
Hi ,
I tried to set the parameter value callingpres and tried making outdial
to the same peer , but the value we set to this parameter is never getting
used when make an
,
Mark Michelson
--
_
-- 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
On July 4, 2014, 3:43 p.m., Matt Jordan wrote:
/branches/12/res/res_pjsip_dialog_info_body_generator.c, lines 152-155
https://reviewboard.asterisk.org/r/3705/diff/1/?file=62059#file62059line152
Doxygen comments on these #defines would be nice, as they aren't
'obvious' sizes (3?
/res_pjsip_dialog_info_body_generator.c
https://reviewboard.asterisk.org/r/3705/#comment22765
I recommend moving this check higher in the function. Eliminating potential
errors early saves you from constructing a partial XML body only to end up
bailing when you can't retrieve the version number.
- Mark Michelson
On July 2
that the front line
is working, at least.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http
mistakes made in these tests, so I figured I'd get those caught by
reviewers early.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE
, with these changes, I did run the gamut of subscription tests in the
testsuite and they all pass. This at least means that there are no detectable
regressions at this point.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided
---
On July 1, 2014, 11:21 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3699/
---
(Updated
---
On July 1, 2014, 11:21 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3699
is working, at least.
Thanks,
Mark Michelson
--
_
-- 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
a level
of indentation.
/trunk/res/res_fax_spandsp.c
https://reviewboard.asterisk.org/r/3666/#comment22561
Use ast_str_buffer() instead of directly accessing the contents of an
ast_str
- Mark Michelson
On June 26, 2014, 7:54 p.m., Jonathan Rose wrote
://reviewboard.asterisk.org/r/3673/#review12358
---
On June 25, 2014, 7:37 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
certainly
some mistakes made in these tests, so I figured I'd get those caught by
reviewers early.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
or
(fingers crossed) I get a ship it!
- Mark Michelson
On June 26, 2014, 10:40 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3673
lookup of the context.
- Mark Michelson
On June 26, 2014, 9:12 p.m., Jonathan Rose wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3650
an unimplemented feature. There are almost certainly
some mistakes made in these tests, so I figured I'd get those caught by
reviewers early.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com
://reviewboard.asterisk.org/r/3659/#comment22494
To save some indentation, you could combine these two if statements into
one compound one:
if (*opcode == AST_WEBSOCKET_PING ast_websocket_write(session,
AST_WEBSOCKET_OPCODE_PONG, *payload, *payload_len)) {
...
}
- Mark
://reviewboard.asterisk.org/r/3624/#comment22495
Same suggestion as the 12 version of this review
- Mark Michelson
On June 25, 2014, 6:39 p.m., Matt Jordan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3649/#review12320
---
Ship it!
Ship It!
- Mark Michelson
On June 25, 2014, 3:19
Farrell wrote:
Actually I would go a step farther and make this minversion: '1.8.0'.
PUSH and UNSHIFT functions existed before 1.8.0 and were broken, so it would
be correct for this test to fail on all previous versions, instead of being
skipped.
Mark Michelson wrote:
Setting
/
Testing
---
The test passes.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http
Diff: https://reviewboard.asterisk.org/r/3628/diff/
Testing
---
Since no new functionality has been added, I just ran the current gamut of
testsuite tests (tests/channels/pjsip/subscriptions). They all pass.
Thanks,
Mark Michelson
/r/3672/#comment22498
Should this also send the FEATURE_DETECTION failure test event?
- Mark Michelson
On June 25, 2014, 7:14 p.m., opticron wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
nothing :)
trunk/tests/test_channel_datastore_features.c
https://reviewboard.asterisk.org/r/3649/#comment22468
Replace the many while (nanosleep...) loops in this file with a single
function call that takes the number of seconds and nanoseconds to sleep.
- Mark Michelson
On June 24, 2014, 6
/http_websocket.h
https://reviewboard.asterisk.org/r/3659/#comment22471
Give these constants AST_ prefixes since they are global.
- Mark Michelson
On June 20, 2014, 4:57 p.m., Matt Jordan wrote:
---
This is an automatically generated e-mail
: false
Event: ResourceListDetailComplete
EventList: Complete
ListItems: 1
All seems well to me.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
: ResourceListDetail
ObjectType: resource_list
ObjectName: mylist
Event: message-summary
ListItem: foo,bar,baz
NotificationBatchInterval: 5000
FullState: false
Event: ResourceListDetailComplete
EventList: Complete
ListItems: 1
All seems well to me.
Thanks,
Mark Michelson
:
} else if (...) {
/trunk/pbx/pbx_config.c
https://reviewboard.asterisk.org/r/3650/#comment22262
I recommend ast_strdupa() here as well.
- Mark Michelson
On June 19, 2014, 4:41 p.m., Jonathan Rose wrote:
---
This is an automatically
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3651/#review12203
---
Ship it!
Ship It!
- Mark Michelson
On June 19, 2014, 5:01
On June 19, 2014, 5:48 p.m., Mark Michelson wrote:
trunk/include/asterisk/bridge_channel.h, lines 645-653
https://reviewboard.asterisk.org/r/3649/diff/1/?file=59854#file59854line645
Add documentation that chan must be locked when calling this.
Ignore this since I covered
/bridge_channel.c
https://reviewboard.asterisk.org/r/3649/#comment22266
Is this cast required? Either way, it's one of the weirder-looking things
I've come across :)
- Mark Michelson
On June 19, 2014, 3:36 p.m., opticron wrote
://reviewboard.asterisk.org/r/3652/#comment22280
This loop is incorrect. I believe it should be:
if (j = 0; j 8; ++j) {
if ((addr[i] j) 1) {
bits++;
}
}
- Mark Michelson
On June 19, 2014, 5:47 p.m., George Joseph wrote
:
https://reviewboard.asterisk.org/r/3648/#review12217
---
On June 19, 2014, 3:45 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
On June 19, 2014, 6:01 p.m., Mark Michelson wrote:
branches/12/main/netsock2.c, lines 156-160
https://reviewboard.asterisk.org/r/3652/diff/1/?file=59870#file59870line156
This loop is incorrect. I believe it should be:
if (j = 0; j 8; ++j) {
if ((addr[i] j
On June 19, 2014, 6:01 p.m., Mark Michelson wrote:
branches/12/main/acl.c, line 683
https://reviewboard.asterisk.org/r/3652/diff/1/?file=59869#file59869line683
It's typically a good idea to avoid ast_strdupa() inside a for loop
since it theoretically could blow the stack
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3628/#review12224
---
On June 18, 2014, 9:12 p.m., Mark Michelson wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3652/#review12231
---
Ship it!
Ship It!
- Mark Michelson
On June 19, 2014, 7:29
/diff/
Testing
---
Since no new functionality has been added, I just ran the current gamut of
testsuite tests (tests/channels/pjsip/subscriptions). They all pass.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation
ListItems: 1
All seems well to me.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http
/tests/realtime/destroy/configs/ast1/extconfig.conf
PRE-CREATION
/asterisk/trunk/lib/python/asterisk/realtime_test_module.py PRE-CREATION
Diff: https://reviewboard.asterisk.org/r/3363/diff/
Testing
---
Thanks,
Mark Michelson
EventList: Complete
ListItems: 1
All seems well to me.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http
://reviewboard.asterisk.org/r/3648/#review12236
---
On June 19, 2014, 9 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r
of
testsuite tests (tests/channels/pjsip/subscriptions). They all pass.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options
On June 13, 2014, 6:28 p.m., Mark Michelson wrote:
/trunk/apps/app_jack.c, line 64
https://reviewboard.asterisk.org/r/3618/diff/1/?file=59647#file59647line64
I saw your comment about requiring a large ringbuffer for dealing with
larger sampling rates, but I don't think a 128K
be removed from the
review.
- Mark Michelson
On June 14, 2014, 10:12 p.m., Dennis Guse wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3618
---
On June 13, 2014, 9:01 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3619/
---
(Updated June 13, 2014, 9
with this patch applied.
I am working on an official testsuite test, but I am currently having
difficulties with the testsuite.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3397/#review12178
---
Ship it!
Ship It!
- Mark Michelson
On June 6, 2014, 5:26
the if.
trunk/apps/app_queue.c
https://reviewboard.asterisk.org/r/3607/#comment22232
This line can be deleted.
- Mark Michelson
On June 11, 2014, 10:04 a.m., Michael K. wrote:
---
This is an automatically generated e-mail. To reply
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3363/#review12051
---
On March 15, 2014, 6:34 p.m., Mark Michelson wrote
://reviewboard.asterisk.org/r/3363/#review11516
---
On March 15, 2014, 6:34 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
/configs/ast1/sorcery.conf PRE-CREATION
/asterisk/trunk/tests/realtime/destroy/configs/ast1/extconfig.conf
PRE-CREATION
/asterisk/trunk/lib/python/asterisk/realtime_test_module.py PRE-CREATION
Diff: https://reviewboard.asterisk.org/r/3363/diff/
Testing
---
Thanks,
Mark Michelson
-config.yaml PRE-CREATION
/asterisk/trunk/tests/funcs/func_push/configs/ast1/extensions.conf
PRE-CREATION
Diff: https://reviewboard.asterisk.org/r/3619/diff/
Testing
---
The test passes.
Thanks,
Mark Michelson
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3620/#review12145
---
Ship it!
Ship It!
- Mark Michelson
On June 13, 2014, 3:58
to go with
that, at least for the time being.
- Mark Michelson
On June 13, 2014, 12:19 p.m., Dennis Guse wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3618
---
On June 13, 2014, 2:50 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3619/
---
(Updated June 13, 2014, 2
---
The test passes.
Thanks,
Mark Michelson
--
_
-- 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
difficulties with the testsuite.
Thanks,
Mark Michelson
--
_
-- 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
501 - 600 of 927 matches
Mail list logo
