On Thu, Aug 06, 2015 at 03:33:41PM +0300, Oleg Ponomarev wrote:
> This patch consist of two parts:
> 
> * change Statistics.hs introducing Stat typeclass in order to unify
>   work with Statistics. This also allow not to write empty stabs for
>   unwanted types.
> 
> * reimplement Metrics.hs in the type safe manner. Also split
>   the implementation into several files:
>   1) MetricsComponents.hs file contains  MetricComponent data type
>      describes a metrics component. Each metrics component is
>      represented by a variable of type MetricComponent.
>   2) MetricsTH.hs file generates functions dealing with cluster
>      statistics from given metricComponents list using TemplateHaskell
>   3) Metrics.hs file contains the others non-template functions and
>      provides the rest interface
> 
> Signed-off-by: Oleg Ponomarev <[email protected]>
> ---
>  Makefile.am                                     |   2 +
>  src/Ganeti/HTools/Cluster/AllocatePrimitives.hs |   6 +-
>  src/Ganeti/HTools/Cluster/Metrics.hs            | 163 ++-----------
>  src/Ganeti/HTools/Cluster/MetricsComponents.hs  | 291 
> ++++++++++++++++++++++++
>  src/Ganeti/HTools/Cluster/MetricsTH.hs          | 237 +++++++++++++++++++
>  src/Ganeti/Utils/Statistics.hs                  | 180 ++++++++-------
>  test/hs/Test/Ganeti/Utils/Statistics.hs         |   4 +-
>  7 files changed, 641 insertions(+), 242 deletions(-)
>  create mode 100644 src/Ganeti/HTools/Cluster/MetricsComponents.hs
>  create mode 100644 src/Ganeti/HTools/Cluster/MetricsTH.hs

I think you added a few last-minute typos. My suggestion is to add the
following interdiff (to make it compile). Otherwise LGTM.

commit a3e252b4435e1ed366276a1ebc373272a787b1dd
Author: Klaus Aehlig <[email protected]>
Date:   Thu Aug 6 15:32:06 2015 +0200

    Interdiff

diff --git a/src/Ganeti/HTools/Cluster/MetricsTH.hs 
b/src/Ganeti/HTools/Cluster/MetricsTH.hs
index 566b844..1ec6165 100644
--- a/src/Ganeti/HTools/Cluster/MetricsTH.hs
+++ b/src/Ganeti/HTools/Cluster/MetricsTH.hs
@@ -140,8 +140,8 @@ getQNameExp n e = do
 compClusterStatisticsDecl :: [MetricComponent] -> Q [Dec]
 compClusterStatisticsDecl components = do
   nl_i <- newName "nl"
-  let splitted = appTwice [| partition |] [| Node.offline |] nl
-      (nl_off, nl_on) = (appE [| fst |] partitioned, appE [| snd |] splitted)
+  let splitted = appTwice [| partition |] [| Node.offline |] (varE nl_i)
+      (nl_off, nl_on) = (appE [| fst |] splitted, appE [| snd |] splitted)
       (online, offline) = partition forOnlineNodes components
       nv_f nm = varE . mkName $ "nv_" ++ nm
       nvl_f = appTwice [| map |] (varE (mkName "getNodeValues"))

If you agree (please state in an email if you do), I will squash the interdiff
into your patch and push the change.

Thanks for your patch!

Regards,
Klaus

-- 
Klaus Aehlig
Google Germany GmbH, Dienerstr. 12, 80331 Muenchen
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores

Reply via email to