On Tue, 2009-04-07 at 12:14 -0400, Damian Krzeminski wrote: > http://track.sipfoundry.org/browse/XCF-3413 > > I made some changes on top of David's patch in the areas concerning > sipXconfig. > > Looking for a volunteer to review the non-sipXconfig part and commit it all > (or letting me know that it works after the review so that I can commit it).
The ZoneAdminRpc.h file should contain a Doxygen description on the ZoneAdminRpcExec class that provides a detailed description of the XML RPC method it implements. See ImdbRpc.h (class ImdbRpcReplaceTable) for a good example (and http://sipxecs.sipfoundry.org/doc/sipxsupervisor/class_imdb_rpc_replace_table.html for the resulting document). I'd have put the source and installation of sipxzoneadmin and its setup script into sipXpbx (rather than sipXcommserverLib) so that it's in the same project with the shell script. Not a big deal, but I think it would be less confusing to someone trying to find all the moving parts. I like to see the changes to initial-config done differently. It should not dynamically determine it's domain name or ip address; if that information is needed, it should be added as option arguments and passed in by sipXconfig. That should be changed for myHostname as well while we're at it. Some general shell script advice: - Don't use 'grep' to do string compares: myDomname=`dnsdomainname` myZonedns=`host -t NS ${myDomname}` echo "$myZonedns" | grep "$myHostname" if [ $? -ne 0 ] is better written as: myDomname=`dnsdomainname` myZonedns=`host -t NS ${myDomname}` echo "$myZonedns" | grep "$myHostname" if [ "${myDomname}" = "${myZonedns}" ] - Also on the above, use the 'dns_ns' function provided by sipx-utils.sh rather than the 'host' command to get the server for the domain. - When you want to write a file, use a here-document rather than a series of separate commands, so this: { printf "options {\n" printf " directory \"/var/named\";\n" printf " dump-file \"/var/named/data/cacahe_dump.db\";\n" printf " statistics-file \"/var/named/data/named_stats.txt\";\n" printf "};\n" printf "\n" printf "zone \"$myDomname\" IN {\n" printf " type slave;\n" printf " file \"$myDomname.zone\";\n" printf " masters {$myHostaddr;};\n" printf "};\n" } > "${INITIAL_CONFIG}/etc/named.conf" should be: cat > "${INITIAL_CONFIG}/etc/named.conf" <<EOF options { directory "/var/named"; dump-file "/var/named/data/cacahe_dump.db"; statistics-file "/var/named/data/named_stats.txt"; }; zone "$myDomname" IN { type slave; file "$myDomname.zone"; masters {${myHostaddr};}; }; EOF the later form is much easier to read (even more so than seen above when seen in an editor that does proper syntax-coloring for shell scripts). It also is much easier to get the escaping right, because it's almost never needed at all. - Always put shell variables in ${}: Use ${myHostaddr} rather than $myHostaddr even when you don't have to; it's just a good habit and will save you some difficult-to-spot errors someday. _______________________________________________ sipx-dev mailing list [email protected] List Archive: http://list.sipfoundry.org/archive/sipx-dev Unsubscribe: http://list.sipfoundry.org/mailman/listinfo/sipx-dev
