On Tue, Feb 04, 2014 at 11:12:50AM +0100, Michele Tartara wrote:
> On Fri, Jan 31, 2014 at 11:46 AM, Jose A. Lopes <[email protected]> wrote:
> > The monitoring daemon module defines a type alias 'PrepResult' for
> > 'Config Snap ()', so we can use it instead of the type.
> >
> > Signed-off-by: Jose A. Lopes <[email protected]>
> > ---
> >  src/Ganeti/Monitoring/Server.hs | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/Ganeti/Monitoring/Server.hs 
> > b/src/Ganeti/Monitoring/Server.hs
> > index 093f2c1..b193c33 100644
> > --- a/src/Ganeti/Monitoring/Server.hs
> > +++ b/src/Ganeti/Monitoring/Server.hs
> > @@ -6,7 +6,7 @@
> >
> >  {-
> >
> > -Copyright (C) 2013 Google Inc.
> > +Copyright (C) 2013, 2014 Google Inc.
> >
> >  This program is free software; you can redistribute it and/or modify
> >  it under the terms of the GNU General Public License as published by
> > @@ -100,7 +100,7 @@ collectors =
> >  -- * Configuration handling
> >
> >  -- | The default configuration for the HTTP server.
> > -defaultHttpConf :: FilePath -> FilePath -> Config Snap ()
> > +defaultHttpConf :: FilePath -> FilePath -> PrepResult
> >  defaultHttpConf accessLog errorLog =
> >    setAccessLog (ConfigFileLog accessLog) .
> >    setCompression False .
> > --
> > 1.8.5.3
> >
> 
> I don't think this change is good.
> 
> PrepResult is a type that is used for daemons in Ganeti, whereas
> Config Snap () is a type specific to the Snap library.

PrepResult is defined in the same module, at the top, and it seems
that other daemons define their own PrepResult type alias, for
example, the query daemon.  So, I would say this particular PrepResult
is Snap specific, especially given that it's type contains Snap:

  -- | Type alias for prepMain results.
  type PrepResult = Config Snap ()

Is it possible you are confusing PrepResult with PrepFn?

> Here, defaultHttpConf is something that is snap-specific, so I think
> it should probably stay as it is, and be renamed PrepResult only as
> part of the prepMain function, as it happens already.
> 
> Thanks,
> Michele
> -- 
> Google Germany GmbH
> Dienerstr. 12
> 80331 München
> 
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschäftsführer: Graham Law, Christine Elizabeth Flores

-- 
Jose Antonio Lopes
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370

Reply via email to