Conversation
|
Current dependencies on/for this PR:
This stack of pull requests is managed by Graphite. |
| @@ -11,10 +11,10 @@ | |||
| (log/debug "Starting raft consensus with config:" config) | |||
| (let [handler (consensus-handler/create-handler consensus-handler/default-routes)] | |||
| (consensus/start handler (assoc config :join? false | |||
There was a problem hiding this comment.
i'm not sure if cljfmt can handle this, but i think multi-line assoc calls should have their first key-value pair start on a new line so that all the k-v pairs line up for easy scanning. here, the :join? false is on a different column from all the other pairs, so someone doing a quick scan to see which attributes are set to one could easily miss it.
There was a problem hiding this comment.
Yeah, I don't think it can enforce that.
| (io-file/connection-storage-read | ||
| log-directory | ||
| encryption-key)) | ||
| log-directory |
There was a problem hiding this comment.
i think i prefer 2-space indentation for function calls when all the arguments are on a new line. i think it better expresses the semantic difference between the function and the arguments it takes. but i'm also ok with leaving it this way if other folks disagree.
There was a problem hiding this comment.
This is actually just cljfmt treating lists like other multiline compound data literals. For example:
[:foo
:bar
:baz]I'm not sure if you can tell it to make an exception for lists or if you'd have to make all such data literals indent w/ two spaces on subsequent lines.
There was a problem hiding this comment.
Looks like I was wrong and this is specifically configurable in cljfmt: https://github.com/weavejester/cljfmt#formatting-options (see :function-arguments-indentation section). I'm happy to go w/ whatever the team consensus is here.
There was a problem hiding this comment.
I like 2-space indentation here as well
There was a problem hiding this comment.
Do you all have a preference between the :cursive and :zprint options for that param?
There was a problem hiding this comment.
eh. seeing it so strongly rejected in the style guide (from the link in the description of the "community" default option) is making me rethink my stance on this. perhaps we shouldn't go against the default.
| (invalid-address-shutdown | ||
| (str "Error in multi-address type in: " multi-addr | ||
| " Fluree only supports the ipv4, ipv6, dns4 or dns6 at the moment."))) | ||
| (str "Error in multi-address type in: " multi-addr |
There was a problem hiding this comment.
i think this is an even better example of why 2 space indentation for function calls is more legible.
| @@ -1,30 +1,30 @@ | |||
| (ns fluree.server.components.http | |||
| (:require | |||
There was a problem hiding this comment.
Stuart Sierra notwithstanding, I prefer the first ns require to be on the same line as the :require keyword. I think the extra indentation that gives helps break up the require blocks vs imports, uses, and refer-clojures.
There was a problem hiding this comment.
I tend to agree, but I think this is another thing that cljfmt cannot (yet?) enforce.
Merge activity
|
51e1db8 to
ade62aa
Compare
ade62aa to
db9c944
Compare

No description provided.