[sheepdog] [PATCH v5 0/4] shepherd: a new cluster manager specialized for sheepdog

Hitoshi Mitake h.mitake at gmail.com
Wed Jan 9 16:44:36 CET 2013


At Wed, 09 Jan 2013 15:31:51 +0800,
Liu Yuan wrote:
> 
> On 01/09/2013 03:29 PM, Liu Yuan wrote:
> > On 01/09/2013 03:09 PM, Hitoshi Mitake wrote:
> >>  .gitignore               |    1 +
> >>  Makefile.am              |    2 +-
> >>  configure.ac             |    3 +-
> >>  include/Makefile.am      |    3 +-
> >>  include/internal_proto.h |   51 +++
> >>  include/net.h            |    1 +
> >>  include/shepherd.h       |   78 ++++
> >>  include/shepherd_msg.def |   17 +
> > 
> > I haven't got time to review the internal functions, but for general
> > coding style:
> >  1 should remove extra white space after 'break;'
> >  2 find another better descriptive name for _function_name(). prefix the
> > existing function name tell nothing about different between
> > _function_name() and function_name().
> > 
> > And for shepherd_msg.def, don't use it because it obscure 'cscope' and
> > introduce tricky macros which isn't necessary.
> > 
> > init_msg_handlers() isn't necessary. Refer to zookeeper.c for a static
> > handler installation because you don't need dynamic callback installation.
> > 
> 
> And more, should make shepherd as optional at configure phrase
> 

Current shepherd doesn't require any special libraries. So the build
of shepherd never causes compile errors. Should it be optional?

Thanks,
Hitoshi




More information about the sheepdog mailing list