On Tue, Feb 4, 2014 at 11:43 AM, Jose A. Lopes <[email protected]> wrote:
> 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?

No, I know PrepResult is defined for every daemon independently, and
that they are different. Actually, PrepResult is the return type of
PrepFn (which is called PrepMain in this particular daemon), and
that's my point: PrepResult should be returned by PrepMain.
DefaultHttpConf is another function, returning something else, that
just happens to have a type that is compatible. It's up to PrepMain to
get this "something", modify it a bit more, and then return it as an
actual PrepResult.

>
>> 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

Cheers,
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

Reply via email to