Skip to content
This repository has been archived by the owner on Mar 18, 2019. It is now read-only.

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

Open
lvh opened this issue Feb 2, 2016 · 2 comments
Open

Comments

@lvh
Copy link
Contributor

lvh commented Feb 2, 2016

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:

  • By the time we get to this ticket, someone has already solved this somehow in kibit (check!)
  • Amend kibit to conditionally enable/disable some checks, or make some of them non-gating. This potentially adds a lot of complexity to kibit, though; and we don't know if upstream agrees that that's a good thing.
  • Amend kibit to maintain a list of exceptions to that rule, i.e. it's OK if it's md/catch. This has limitations; AFAIK kibit doesn't really import your code so it has to count on statically resolvable things to figure out what a particular fn is; and, right now, kibit works in a "peephole" fashion (that is, it only looks at a form at a time, so it would e.g. not be easy or pleasant to also look at an ns form).
  • Workaround: assign the "happy case" deferred to a name in a let binding (not very good) or deal with the inside-out form (not very good).
@lvh
Copy link
Contributor Author

lvh commented Feb 3, 2016

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.

@lvh lvh mentioned this issue Feb 3, 2016
@ehashman
Copy link
Contributor

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 clj-commons/kibit#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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants