RackSec / desdemona

Data-backed security operations
Eclipse Public License 1.0
2 stars 7 forks source link

Kibit gates on two-form thread exprs (->, ->>) which is not always desirable #16

Open lvh opened 8 years ago

lvh commented 8 years ago

Kibit has rules about two-form thread exprs. It complains about things like (->> f a) and (-> f a) and tells you to rewrite them as (f a). That certainly looks reasonable, but sometimes that is really the treading macro you want. For example, consider manifold.deferred's catch, which tells you to:

(-> (form that throws)
    (md/catch Exception (handler))

That's what upstream thinks looks nicest, and there appears to be consensus on our development team as well (@derwolfe, @lvh). The desugared form, (md/catch (form that throws) Exception (handler)), while obviously equivalent, is harder to read and understand; you want to think about what you're doing first before you think about error handling. The problem goes away when you add more fns to that pipeline; then kibit is happy with the form.

Potential solutions:

lvh commented 8 years ago

kibit just got made non-gating in #22 because @sirsean got bit by it in #21. Obviously a gating CI check is way better than a non-gating check, so we want to re-enable this.

ehashman commented 8 years ago

For what it's worth, the kibit maintainers seem to agree that it would be good to make the rulesets more configurable and allow for the exclusion of certain rules (see https://github.com/jonase/kibit/issues/148), but none of that code's been written yet. I guess we have the option of writing it ourselves, with the bonus of supporting upstream.