Merge pull request #272 from metabase/add_bikeshed_and_uberjar

add bikeshed linter and build uberjar in circle

circle.yml

@@ -2,10 +2,7 @@ machine:
   java:
     version:
       oraclejdk8
-dependencies:
-  override:
-    - lein deps # We can skip inferred `npm install` since we're not running browser tests (yet)
 test:
   override:
-    - case $CIRCLE_NODE_INDEX in 0) lein eastwood ;; 1) lein test ;; esac:
+    - case $CIRCLE_NODE_INDEX in 0) lein eastwood ;; 1) lein test ;; 2) lein bikeshed --max-line-length 240 && lein uberjar ;; esac:
         parallel: true

project.clj

@@ -38,7 +38,7 @@
                                com.sun.jmx/jmxri]]
                  [medley "0.5.5"]                                     ; lightweight lib of useful functions
                  [org.liquibase/liquibase-core "3.3.2"]               ; migration management (Java lib)
-                 [org.slf4j/slf4j-log4j12 "1.7.10"]
+                 [org.slf4j/slf4j-log4j12 "1.7.12"]
                  [org.yaml/snakeyaml "1.15"]                          ; YAML parser (required by liquibase)
                  [postgresql "9.3-1102.jdbc41"]                       ; Postgres driver
                  [ring/ring-jetty-adapter "1.3.2"]                    ; Ring adapter using Jetty webserver (used to run a Ring server for unit tests)
@@ -56,13 +56,14 @@
   :eastwood {:exclude-namespaces [:test-paths]
              :add-linters [:unused-private-vars]
              :exclude-linters [:constant-test]}                       ; korma macros generate some formats with if statements that are always logically true or false
-  :profiles {:dev {:dependencies [[org.clojure/tools.nrepl "0.2.8"]   ; REPL <3
+  :profiles {:dev {:dependencies [[org.clojure/tools.nrepl "0.2.10"]  ; REPL <3
                                   [expectations "2.1.0"]              ; unit tests
                                   [marginalia "0.8.0"]                ; for documentation
                                   [ring/ring-mock "0.2.0"]]
                    :plugins [[cider/cider-nrepl "0.9.0-SNAPSHOT"]     ; Interactive development w/ cider NREPL in Emacs
                              [jonase/eastwood "0.2.1"]                ; Linting
                              [lein-ancient "0.6.5"]                   ; Check project for outdated dependencies + plugins w/ 'lein ancient'
+                             [lein-bikeshed "0.2.0"]                  ; Linting
                              [lein-expectations "0.0.8"]              ; run unit tests with 'lein expectations'
                              [lein-instant-cheatsheet "2.0.0"]        ; use awesome instant cheatsheet created by yours truly w/ 'lein instant-cheatsheet'
                              [lein-marginalia "0.8.0"]]               ; generate documentation with 'lein marg'

src/metabase/api/common.clj

@@ -85,13 +85,13 @@
 
     (check test1 code1 message1
            test2 code2 message2)"
-  ([test code-or-code-message-pair & rest-args]
+  ([tst code-or-code-message-pair & rest-args]
    (let [[[code message] rest-args] (if (vector? code-or-code-message-pair)
                                       [code-or-code-message-pair rest-args]
                                       [[code-or-code-message-pair (first rest-args)] (rest rest-args)])]
-     (when-not test
+     (when-not tst
        (throw (ApiException. (int code) message)))
-     (if (empty? rest-args) test
+     (if (empty? rest-args) tst
          (recur (first rest-args) (second rest-args) (drop 2 rest-args))))))
 
 (defn check-exists?
@@ -175,15 +175,15 @@
 
     (api-> [404 \"Not found\"] @*current-user*
       :id)"
-  [response-pair test & body]
-  `(api-let ~response-pair [result# ~test]
+  [response-pair tst & body]
+  `(api-let ~response-pair [result# ~tst]
      (-> result#
          ~@body)))
 
 (defmacro api->>
   "Like `api->`, but threads result using `->>`."
-  [response-pair test & body]
-  `(api-let ~response-pair [result# ~test]
+  [response-pair tst & body]
+  `(api-let ~response-pair [result# ~tst]
      (->> result#
           ~@body)))
 
@@ -193,14 +193,14 @@
 
 ;; #### GENERIC 400 RESPONSE HELPERS
 (def generic-400 [400 "Invalid Request."])
-(defn     check-400 [test]    (check test generic-400))
+(defn     check-400 [tst]     (check tst generic-400))
 (defmacro let-400   [& args] `(api-let   ~generic-400 ~@args))
 (defmacro ->400     [& args] `(api->     ~generic-400 ~@args))
 (defmacro ->>400    [& args] `(api->>    ~generic-400 ~@args))
 
 ;; #### GENERIC 404 RESPONSE HELPERS
 (def generic-404 [404 "Not found."])
-(defn     check-404 [test]    (check test generic-404))
+(defn     check-404 [tst]     (check tst generic-404))
 (defmacro let-404   [& args] `(api-let   ~generic-404 ~@args))
 (defmacro ->404     [& args] `(api->     ~generic-404 ~@args))
 (defmacro ->>404    [& args] `(api->>    ~generic-404 ~@args))
@@ -208,7 +208,7 @@
 ;; #### GENERIC 403 RESPONSE HELPERS
 ;; If you can't be bothered to write a custom error message
 (def generic-403 [403 "You don't have permissions to do that."])
-(defn     check-403 [test]    (check test generic-403))
+(defn     check-403 [tst]     (check tst generic-403))
 (defmacro let-403   [& args] `(api-let   ~generic-403 ~@args))
 (defmacro ->403     [& args] `(api->     ~generic-403 ~@args))
 (defmacro ->>403    [& args] `(api->>    ~generic-403 ~@args))
@@ -216,7 +216,7 @@
 ;; #### GENERIC 500 RESPONSE HELPERS
 ;; For when you don't feel like writing something useful
 (def generic-500 [500 "Internal server error."])
-(defn     check-500 [test]    (check test generic-500))
+(defn     check-500 [tst]     (check tst generic-500))
 (defmacro let-500   [& args] `(api-let   ~generic-500 ~@args))
 (defmacro ->500     [& args] `(api->     ~generic-500 ~@args))
 (defmacro ->>500    [& args] `(api->>    ~generic-500 ~@args))

src/metabase/models/hydrate.clj

@@ -23,10 +23,10 @@
 ;; ## REALIZE-JSON
 
 (defn- read-json-str-or-clob
-  "If STR is a JDBC Clob, convert to a String. Then call `json/read-str`."
-  [str]
-  (some-> (if-not (= (type str) org.h2.jdbc.JdbcClob) str
-                  (u/jdbc-clob->str str))
+  "If JSON-STRING is a JDBC Clob, convert to a String. Then call `json/read-str`."
+  [json-str]
+  (some-> (if-not (= (type json-str) org.h2.jdbc.JdbcClob) json-str
+                  (u/jdbc-clob->str json-str))
           cheshire/parse-string))
 
 (defn realize-json

src/metabase/setup.clj

 (ns metabase.setup)
 
-
 (def ^:private setup-token
   (atom nil))
 
@@ -21,5 +20,3 @@
   "Clear the `@setup-token` if it exists and reset it to nil."
   []
   (reset! setup-token nil))
-
-

src/metabase/util.clj

@@ -15,10 +15,10 @@
   "Like `select-keys` but filters out key-value pairs whose value is nil.
    Unlike `select-keys`, KEYS are rest args (should not be wrapped in a vector).
    TODO: Why?"
-  [m & keys]
+  [m & ks]
   {:pre [(map? m)
-         (every? keyword? keys)]}
-  (->> (select-keys m keys)
+         (every? keyword? ks)]}
+  (->> (select-keys m ks)
        (filter-vals identity)))
 
 (defmacro fn->
@@ -81,10 +81,10 @@
       (-assoc* ~@kvs))
     ~object))
 
-(defmacro -assoc* [k v & rest]
+(defmacro -assoc* [k v & more]
   `(let [~'<> (assoc ~'<> ~k ~v)]
-        ~(if (empty? rest) `~'<>
-             `(-assoc* ~@rest))))
+        ~(if (empty? more) `~'<>
+             `(-assoc* ~@more))))
 
 (defn new-sql-timestamp
   "`java.sql.Date` doesn't have an empty constructor so this is a convenience that lets you make one with the current date.
@@ -110,8 +110,8 @@
 
 (defn now-with-format
   "format the current time using a custom format."
-  [format]
-  (time/unparse (time/formatter format) (coerce/from-long (System/currentTimeMillis))))
+  [format-string]
+  (time/unparse (time/formatter format-string) (coerce/from-long (System/currentTimeMillis))))
 
 (defn jdbc-clob->str
   "Convert a `JdbcClob` to a `String`."