On Fri, Jun 22, 2012 at 11:32 AM, René Nussbaumer <r...@google.com> wrote:
> On Fri, Jun 22, 2012 at 10:46 AM, Agata Murawska
> <agatamuraw...@google.com> wrote:
>> As hspace and hcheck both use machine readable options, they require
>> similar functions - which are now generalized and moved to CLI and
>> Utils.
>>
>> Signed-off-by: Agata Murawska <agatamuraw...@google.com>
>> ---
>>  htools/Ganeti/HTools/CLI.hs            |   16 +++++++++
>>  htools/Ganeti/HTools/Program/Hspace.hs |   55 
>> ++++++++++++++------------------
>>  htools/Ganeti/HTools/Utils.hs          |    9 +++++-
>>  3 files changed, 48 insertions(+), 32 deletions(-)
>>
>> diff --git a/htools/Ganeti/HTools/CLI.hs b/htools/Ganeti/HTools/CLI.hs
>> index aa4dc2c..308fe6d 100644
>> --- a/htools/Ganeti/HTools/CLI.hs
>> +++ b/htools/Ganeti/HTools/CLI.hs
>> @@ -39,6 +39,8 @@ module Ganeti.HTools.CLI
>>   , maybePrintNodes
>>   , maybePrintInsts
>>   , maybeShowWarnings
>> +  , printKeys
>> +  , printFinal
>>   , setNodeStatus
>>   -- * The options
>>   , oDataFile
>> @@ -83,6 +85,7 @@ module Ganeti.HTools.CLI
>>   ) where
>>
>>  import Control.Monad
>> +import Data.Char (toUpper)
>>  import Data.Maybe (fromMaybe)
>>  import qualified Data.Version
>>  import System.Console.GetOpt
>> @@ -563,6 +566,19 @@ maybeShowWarnings fix_msgs =
>>     hPutStrLn stderr "Warning: cluster has inconsistent data:"
>>     hPutStrLn stderr . unlines . map (printf "  - %s") $ fix_msgs
>>
>> +-- | Format a list of key, value as a shell fragment.
>> +printKeys :: String -> [(String, String)] -> IO ()
>> +printKeys prefix = mapM_ (\(k, v) ->
>> +                       printf "%s_%s=%s\n" prefix (map toUpper k) 
>> (ensureQuoted v))
>> +
>> +-- | Prints the final @OK@ marker in machine readable output.
>> +printFinal :: String -> Bool -> IO ()
>
> It's confusing if you don't know that the Bool param decides that the
> output is machine readable, can you please document this?
Sure

>
> [...]
>> +  printKeysHTS $ map (\(x, y) -> (printf "ALLOC_%s_CNT" (show x),
>>                                printf "%d" y)) sreason
>
> Just for the sake of better readability, can you realign the second
> line here? :) Here and in other places where you just replace the Name
> :)
Ack.

Note: these changes will be send as a separate patch.

Agata

Reply via email to