BIND 10 #2954: DHCP-DDNS: Create BIND 10 process skeleton

[BIND 10 Development]

#2954: DHCP-DDNS: Create BIND 10 process skeleton
-------------------------------------+-------------------------------------
            Reporter:  stephen       |                        Owner:  tmark
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcp          |                    Milestone:
            Keywords:                |  Sprint-DHCP-20130523
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by marcin):

 * owner:  marcin => tmark


Comment:

 Reviewed commit e448bbba3ecae68b261612954aa9777edc384be4

 I have made this comment some time ago via email. I believe that in D2Srv,
 the !''Srv!'' is redundant or even wrong. Please note that none of the
 modules in BIND10 are called like this. For example, b10-dhcp4 is indeed a
 server process but it is called b10-dhcp4. Perhaps, simply using d2 would
 be sufficient. If we are not concerned about the length of the name of
 this module, we could even try !''dhcpddns!''. Even in this stub
 implementation, there is a lot of !''Srv!'' in the code already.
 Programmers are lazy by their nature, so writing !''Srv!'' everywhere
 might be a pain for some people :-).

 '''b10-d2srv.xml'''
 Copyright date should be corrected in file header.

 Errors in the following tags:
 - refentryinfo - invalid date
 - refnamediv - refpurpose is invalid - copied from DHCP module
 - docinfo - invalid date
 - cmdsynopsis - the (-v) option is not supported right now, so there is no
 reason to document it.
 - refsynopsisdiv - same as above with -v option

 '''d2srv_log.cc'''
 Invalid copyright date.

 Comment before !''include!'' does not match the module. We define the
 logger for !''d2!'', not b10-dhcp4.

 '''d2srv_log.h'''
 Copyright date is invalid.
 Comment before !''extern!'' is copy-pasted from DHCP4 and thus invalid in
 this context.

 '''d2srv_messages.mes'''
 Invalid copyright date.

 D2SRV_STARTING & D2SRV_START_INFO: Shouldn't it be !''D2Controller!'',
 instead of !''DController!''?

 D2SRV_START_INFO: Do we really plan to have the ability to run the server
 in a standalone mode? If we do, how do we get the configuration?
 BTW, if we decide that we need it (for example to run unit tests), it
 should be documented in b10-d2srv.xml?

 '''main.cc'''
 Invalid copyright date.

 usage(): what is the [-p number] parameter? It is not described anywhere,
 including the error message printed by the usage(). In fact, any attempt
 to specify this parameter would result in error and usage printing.

 Shouldn't verbose mode be only available when standalone is turned on? We
 control the verbosity by other means, i.e. using bindctl if running a
 module using bind10.

 main(): Why define the !''ret!'' variable? You could do:
 {{{
 return (EXIT_SUCCESS);
 }}}

 '''Makefile.am'''
 Do we really have to link with all these libraries already? I would be in
 favor to link against the minimal number of libraries and extend the
 dependencies as we go. The libb10-dhcp++.la is already linked with the
 number of libs, so I don't know whether explicitly adding these is really
 needed.

 '''spec_config.h.pre'''
 Copyright date is invalid.

 The location of the spec file is also invalid.

 '''tests/Makefile.am'''
 The same comment regarding linking with many libraries as above.

 '''tests/d2srv_unittests.cc'''
 Copyright date in the header is invalid.

 '''tests/d2srv_test.py'''
 Copyright date is invalid.

 Do we actually have a ticket to implement !''test_alive!''? Currently the
 comment says it is just a placeholder.

 '''!ChangeLog'''
 If we wanted to follow the convention from the !ChangeLog we would rather
 say:
 {{{
 6xx. [func] tmark
     b10-d2srv: Initial DHCP-DDNS (a.k.a. D2) module implemented. Currently
 it does
     nothing useful, except for providing the skeleton implementation
     to be expanded in the future.
     (Trac #2954, git TBD)
 }}}

-- 
Ticket URL: <http://bind10.isc.org/ticket/2954#comment:4>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development