Seagate / halon

High availability solution
Apache License 2.0
1 stars 0 forks source link

cep/engine: minor cosmetics #1466

Closed 1468ca0b-2a64-4fb4-8e52-ea5806644b4c closed 5 years ago

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: andriytk

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: vvv

Is Execute not needed any more?

vvv commented 5 years ago

closed

chumakd commented 5 years ago

assigned to @valery.vorotyntsev

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: andriytk

Ok, done.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: andriytk

RuleData is referenced later after the Request. Mode has no meaning by itself without the Request.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: andriytk

It improves the reference locality.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: andriytk

To improve the Reference locality. RuleData is referenced later, after the Request. The Mode is also referenced later at Request. By itself it has no meaning (mode of what?).

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: andriytk

Pardon, what is wrong here?

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: andriytk

Ok, fixed.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: andriytk

What exactly?

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: andriytk

For me it is a lot! It allows instantly and easily see the difference in the return types of different constructors.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: andriytk

Seems so, I could not find where it is used.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: vvv

OK.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: vvv

Good. Consider putting a space between m and {.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: vvv

Yes, the documentation is better this way.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: vvv

Please provide the reason for relocating RuleData and Mode.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: vvv

How relocation of RuleData improves the code?

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: vvv

Indentation.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: vvv

Please use 2 or 4 spaces for indentation. Odd number of spaces is... odd.

Most of Halon code uses 2 space indentation — the default value used by Emacs' haskell-mode. I find 2 spaces to be too narrow and use 4 [*]: Haskell code is dense, narrow indentations hinder the readability. (Linux kernel uses 8-chars indentation for a good reason.)

[*] I use 2-chars indentation for where lines, records, lists... E.g.

foo = bar
  where  -- indented with 2 spaces
    bar = undefined  -- 4 spaces of indentation

data Rec = Rec
  { f1 :: Int  -- 2 spaces of indentation, but the field itself starts at column 4
  , f2 :: Char
  } deriving Show

data T
  = T1
  | T2 Float
  | T3 (Text, Int)
  deriving (Eq, Show)

See also

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: vvv

What's the point?

Please revert.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: vvv

[optional] Squeezing excessive spaces into one would be an improvement though.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 6 years ago

Created by: vvv

Please don't. This group of changes does not improve readability.