[Asterisk-code-review] res_redisd: add REDIS support modules to send asterisk DeviceState up... (asterisk[master])

[Alec Davis]

Alec Davis has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18828 )

Change subject: res_redisd: add REDIS support modules to send asterisk DeviceState updates to a REDIS server
......................................................................


Patch Set 1:

(14 comments)

Patchset:

PS1: 
Thanks for the feedback


File configs/samples/redisd.conf.sample:

https://gerrit.asterisk.org/c/asterisk/+/18828/comment/73d91911_8328f64d 
PS1, Line 1: [general]
> The sample config is a little sparse, though maybe there isn't really much to say. […]
Ack


File include/asterisk/redisd.h:

https://gerrit.asterisk.org/c/asterisk/+/18828/comment/db4e41d9_672cf4d7 
PS1, Line 6:  * David M. Lee, II <dlee at digium.com>
> Copyright looks wrong? Is this file really his or yours?
Done


File res/res_redis_device_state.c:

https://gerrit.asterisk.org/c/asterisk/+/18828/comment/2e4f1600_7abf98fd 
PS1, Line 4:  * Copyright (C) 2005-2006, Digium, Inc.
> Remove this - copyright is yours not Digium's
Done


https://gerrit.asterisk.org/c/asterisk/+/18828/comment/056cd48a_b2b01624 
PS1, Line 80:     //throw away result
> Use /* comments */ only. […]
Done


https://gerrit.asterisk.org/c/asterisk/+/18828/comment/46b37d89_67eca4b0 
PS1, Line 81:     char return_buffer[4096];
> Did you compile in developer mode? Mixed variables and declarations are *not* permitted.
no, never used it before, and that's been many commits to asterisk.

so I should be using ./configure --enable-dev-mode ?


File res/res_redisd.c:

https://gerrit.asterisk.org/c/asterisk/+/18828/comment/bff11df5_733917d3 
PS1, Line 4:  * Copyright (C) 2005-2006, Digium, Inc.
> Remove this. The copyright is yours, not Digium's.
Done


https://gerrit.asterisk.org/c/asterisk/+/18828/comment/a90093e9_dec93fc9 
PS1, Line 27:  *
> formatting seems messed up here?
Done


https://gerrit.asterisk.org/c/asterisk/+/18828/comment/644ed7d6_02765ee8 
PS1, Line 47: 
> It seems like you're using spaces everywhere, not tabs. […]
Done


https://gerrit.asterisk.org/c/asterisk/+/18828/comment/a1dd82e6_14821a3e 
PS1, Line 58:                 <synopsis>Global configuration settings</synopsis>
> For modules that have default options, explicitly specify them here using the appropriate XML attrib […]
Done


https://gerrit.asterisk.org/c/asterisk/+/18828/comment/4e1a9058_f395349a 
PS1, Line 139:     if (redisd_context == NULL) {
> You could just do !redisd_context
Was following example.c on redis example.
https://github.com/redis/hiredis/blob/master/examples/example.c


https://gerrit.asterisk.org/c/asterisk/+/18828/comment/dc249134_e2618473 
PS1, Line 168:         ast_log(LOG_DEBUG, "Authenticated.\n");
> Use ast_debug(level, fmt, ...) not ast_log(LOG_DEBUG, fmt, ... […]
Done


https://gerrit.asterisk.org/c/asterisk/+/18828/comment/06ba157e_d49bc66c 
PS1, Line 248:         
> redness (trailing whitespace)
Ack


https://gerrit.asterisk.org/c/asterisk/+/18828/comment/d1e2e50d_17b973f0 
PS1, Line 305:  
> redness
Ack



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18828
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I17dc11b1d0b4d046a3f092ddf7778108dbcd1cce
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 1
Gerrit-Owner: Alec Davis <alec at bdt.co.nz>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: N A <mail at interlinked.x10host.com>
Gerrit-Comment-Date: Thu, 21 Jul 2022 22:25:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: N A <mail at interlinked.x10host.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220721/ce18bc7e/attachment.html>