BIND 10 #301: Review result for auth

[BIND 10 Development]

#301: Review result for auth
------------------------------+---------------------------------------------
      Reporter:  zhanglikun   |        Owner:  jinmei
          Type:  enhancement  |       Status:  closed
      Priority:  major        |    Milestone:        
     Component:  b10-auth     |   Resolution:  fixed 
      Keywords:               |    Sensitive:  0     
Estimatedhours:  0.0          |        Hours:  0     
      Billable:  1            |   Totalhours:  3.0   
      Internal:  0            |  
------------------------------+---------------------------------------------
Changes (by jinmei):

  * status:  reviewing => closed
  * resolution:  => fixed


Comment:

 Replying to [comment:6 stephen]:
 > >> src/bin/auth/auth_srv.cc
 > >> With very similar names (!AuthSrv and !AuthSrvImpl), it might be
 clearer to put the
 > >> declaration and implementation of !AuthSrvImpl into separate files.
 > > I'm not sure if I understand this, but since !AuthSrvImpl is the
 implementation class
 > > for !AuthSrv! using the pimpl idiom, we cannot separate !AuthSrvImpl
 into a different
 > > .cc file. I noted it's for pimpl in auth_src.h.
 > I was only suggesting that the contents of the auth_srv.cc file be split
 into multiple files - e.g. put the definition of !AuthSrvImpl in
 auth_srv_impl.h and the methods in auth_srv_impl.cc and do appropriate
 #includes.  The pimpl idiom - which in this case is that auth_srv.h is
 impervious to changes in the functionality of !AuthSrv(Impl) - is
 preserved.
 >
 Ah, okay.  Hmm, we may need this separation if a module (with full
 implementation) gets very large (although in that case we may have to
 reconsider the class design), but in this case I think it's a matter of
 preference.  And,

 > However, it's only a style suggestion and there is no need to make any
 changes.
 >
 you seem to agree with this, so for now I'll keep the unified style.

 [snip other points]

 > All other changes are OK.
 >
 > Please go ahead and merge.
 >
 Okay, done.  I won't bother to consume a ChangeLog entry for this ticket.

 Thanks for the review.

-- 
Ticket URL: <https://bind10.isc.org/ticket/301#comment:7>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development