search

scrive / log

Structured logging solution.
https://hackage.haskell.org/package/log-base
BSD 3-Clause "New" or "Revised" License
35 stars 7 forks source link

Use extra instead of cond #15

Closed phadej closed 8 years ago

phadej commented 8 years ago

This is very opinionated. My gut feeling that extra is transitive dependency more often, and there are other useful extras - I didn't explore the code to find possible use cases though.

23Skidoo commented 8 years ago

I'd just get rid of this dependency by either inlining unlessM or replacing the single use of it with if-then-else.

phadej commented 8 years ago

I could inline unlessM too. There are other useful helpers in extra, but if you don't want to explore what's there, it's fully ok with me (e.g. I needed Data.List.Extra (chunksOf) more than once)

phadej commented 8 years ago

Added a commit with inlined unlessM, feel free to choose / squash

arybczak commented 8 years ago

I don't think either option is a good idea.

1) cond is a small package specialized for Bool related stuff, so it fits. On the other hand, extra brings back a lot of stuff of which anything apart from unlessM is not needed, so it doesn't make sense to replace the one with the other.

2) Copying functionality over from one package to the other just to avoid a dependency is a trap that people fall into far too often. I'd understand if we were talking about avoiding to depend on something of the size of lens, but cond is small and doesn't bring any transitive dependencies.

phadej commented 8 years ago

@arybczak I don't object that.

I just got suspicious to see cond in the dependency list, as it offers Boolean class (which IMHO isn't always the right one, but I maintain lattices ;) ). Where extra is clearly a "stuff that could live in the dependency packages directly".

So in a sense cond does brings stuff which isn't needed too, but different stuff. If you think that cond's extra stuff is "better", I'm ok with that. IMHO extra is effectively as lightweight dependency as cond; it works on windows too, unix is a conditional dependency.

23Skidoo commented 8 years ago

Closing in favour of 8c996fd6a4a73ed85fefc311c9741e16ee0cec23.