More docstrings

src/metabase/api/common.clj

@@ -176,33 +176,33 @@
 
 ;; #### GENERIC 400 RESPONSE HELPERS
 (def ^:private ^:const generic-400 [400 "Invalid Request."])
-(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))
+(defn     check-400 "Throw a 400 if TEST is false."                                                         [tst]    (check tst generic-400))
+(defmacro let-400   "Bind a form as with `let`; throw a 400 if it is `nil` or `false`."                     [& body] `(api-let   ~generic-400 ~@body))
+(defmacro ->400     "If form is `nil` or `false`, throw a 400; otherwise thread it through BODY via `->`."  [& body] `(api->     ~generic-400 ~@body))
+(defmacro ->>400    "If form is `nil` or `false`, throw a 400; otherwise thread it through BODY via `->>`." [& body] `(api->>    ~generic-400 ~@body))
 
 ;; #### GENERIC 404 RESPONSE HELPERS
 (def ^:private ^:const generic-404 [404 "Not found."])
-(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))
+(defn     check-404 "Throw a 404 if TEST is false."                                                         [tst]    (check tst generic-404))
+(defmacro let-404   "Bind a form as with `let`; throw a 404 if it is `nil` or `false`."                     [& body] `(api-let   ~generic-404 ~@body))
+(defmacro ->404     "If form is `nil` or `false`, throw a 404; otherwise thread it through BODY via `->`."  [& body] `(api->     ~generic-404 ~@body))
+(defmacro ->>404    "If form is `nil` or `false`, throw a 404; otherwise thread it through BODY via `->>`." [& body] `(api->>    ~generic-404 ~@body))
 
 ;; #### GENERIC 403 RESPONSE HELPERS
 ;; If you can't be bothered to write a custom error message
 (def ^:private ^:const generic-403 [403 "You don't have permissions to do that."])
-(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))
+(defn     check-403 "Throw a 403 if TEST is false."                                                         [tst]     (check tst generic-403))
+(defmacro let-403   "Bind a form as with `let`; throw a 403 if it is `nil` or `false`."                     [& body] `(api-let   ~generic-403 ~@body))
+(defmacro ->403     "If form is `nil` or `false`, throw a 403; otherwise thread it through BODY via `->`."  [& body] `(api->     ~generic-403 ~@body))
+(defmacro ->>403    "If form is `nil` or `false`, throw a 403; otherwise thread it through BODY via `->>`." [& body] `(api->>    ~generic-403 ~@body))
 
 ;; #### GENERIC 500 RESPONSE HELPERS
 ;; For when you don't feel like writing something useful
 (def ^:private ^:const generic-500 [500 "Internal server error."])
-(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))
+(defn     check-500 "Throw a 500 if TEST is false."                                                         [tst]    (check tst generic-500))
+(defmacro let-500   "Bind a form as with `let`; throw a 500 if it is `nil` or `false`."                     [& body] `(api-let   ~generic-500 ~@body))
+(defmacro ->500     "If form is `nil` or `false`, throw a 500; otherwise thread it through BODY via `->`."  [& body] `(api->     ~generic-500 ~@body))
+(defmacro ->>500    "If form is `nil` or `false`, throw a 500; otherwise thread it through BODY via `->>`." [& body] `(api->>    ~generic-500 ~@body))
 
 
 ;;; ## DEFENDPOINT AND RELATED FUNCTIONS
@@ -210,15 +210,14 @@
 
 ;;; ### Arg annotation fns
 
-(defmulti -arg-annotation-fn
-  "*Internal* - don't use this directly.
+(defmulti ^{:doc "*Internal* - don't use this directly.
 
-   Multimethod used internally to dispatch arg annotation functions.
-   Dispatches on the arg annotation as a keyword.
+                  Multimethod used internally to dispatch arg annotation functions.
+                  Dispatches on the arg annotation as a keyword.
 
-    {id Required}
-    -> ((-arg-annotation-fn :Required) 'id id)
-    -> (annotation:Required 'id id)"
+                   {id Required}
+                   -> ((-arg-annotation-fn :Required) 'id id)
+                   -> (annotation:Required 'id id)"} -arg-annotation-fn ; for some reason supplying a docstr the normal way doesn't assoc it with the metadata like we'd expect
   (fn [annotation-kw]
     {:pre [(keyword? annotation-kw)]}
     annotation-kw))
@@ -440,3 +439,6 @@
    obj)
   ([entity id]
    (check-403 (models/can-write? entity id))))
+
+
+(u/require-dox-in-this-namespace)

src/metabase/api/common/internal.clj

@@ -260,7 +260,7 @@
   [[annotation-kw arg-symb]] ; dispatch-fn passed as a param to avoid circular dependencies
   {:pre [(keyword? annotation-kw)
          (symbol? arg-symb)]}
-  `[~arg-symb (~((eval 'metabase.api.common/-arg-annotation-fn) annotation-kw) '~arg-symb ~arg-symb)])
+  `[~arg-symb (~((resolve 'metabase.api.common/-arg-annotation-fn) annotation-kw) '~arg-symb ~arg-symb)])
 
 (defn process-arg-annotations [annotations-map]
   {:pre [(or (nil? annotations-map)

src/metabase/driver.clj

@@ -266,10 +266,10 @@
   "Search Classpath for namespaces that start with `metabase.driver.`, then `require` them and look for the `driver-init`
    function which provides a uniform way for Driver initialization to be done."
   []
+  (log/info "find-and-load-drivers!")
   (doseq [namespce (filter (fn [ns-symb]
                              (re-matches #"^metabase\.driver\.[a-z0-9_]+$" (name ns-symb)))
                            (ns-find/find-namespaces (classpath/classpath)))]
-    (log/info "loading driver namespace: " namespce)
     (require namespce)))
 
 (defn is-engine?

src/metabase/email/messages.clj

@@ -8,8 +8,8 @@
             [metabase.models.setting :as setting]
             [metabase.pulse :as p, :refer [render-pulse-section]]
             [metabase.util :as u]
-            [metabase.util.quotation :as q]
-            [metabase.util.urls :as url]))
+            (metabase.util [quotation :as quotation]
+                           [urls :as url])))
 
 ;; NOTE: uncomment this in development to disable template caching
 ;; (loader/set-cache (clojure.core.cache/ttl-cache-factory {} :ttl 0))
@@ -19,7 +19,7 @@
 (defn send-new-user-email
   "Format and Send an welcome email for newly created users."
   [invited invitor join-url]
-  (let [data-quote   (rand-nth q/quotations)
+  (let [data-quote   (quotation/random-quote)
         company      (or (setting/get :site-name) "Unknown")
         message-body (stencil/render-file "metabase/email/new_user_invite"
                                           {:emailType       "new_user_invite"
@@ -69,7 +69,7 @@
                         "Segment"   url/segment-url)
         add-url       (fn [{:keys [id model] :as obj}]
                         (assoc obj :url (apply (model->url-fn model) [id])))
-        data-quote    (rand-nth q/quotations)
+        data-quote    (quotation/random-quote)
         context       (-> context
                           (update :dependencies (fn [deps-by-model]
                                                   (for [model (sort (set (keys deps-by-model)))
@@ -111,7 +111,7 @@
   (let [images       (atom [])
         body         (apply vector :div (for [result results]
                                           (render-pulse-section (partial render-image images) :include-buttons result)))
-        data-quote   (rand-nth q/quotations)
+        data-quote   (quotation/random-quote)
         message-body (stencil/render-file "metabase/email/pulse"
                                           {:emailType       "pulse"
                                            :pulse           (html body)

src/metabase/query.clj

@@ -2,11 +2,10 @@
   "Functions for dealing with structured queries."
   (:require [clojure.core.match :refer [match]]))
 
-
 (defn- parse-filter-subclause [subclause]
   (match subclause
          ["SEGMENT" (segment-id :guard integer?)] segment-id
-         subclause                                nil))
+         _                                        nil))
 
 (defn- parse-filter [clause]
   (match clause
@@ -21,8 +20,9 @@
          (filter identity)
          set)))
 
-(defn extract-metric-ids [query]
+(defn extract-metric-ids
+  [query]
   (when-let [aggregation-clause (:aggregation query)]
     (match aggregation-clause
       ["METRIC" (metric-id :guard integer?)] #{metric-id}
-      other                                  nil)))
+      _                                       nil)))

src/metabase/util/quotation.clj

 (ns metabase.util.quotation)
 
 
-(def ^:const quotations
+(def ^:const ^:private quotations
   [{:quote "The world is one big data problem."
     :author "Andrew McAfee"}
    {:quote "Data really powers everything that we do."

test/metabase/api/notify_test.clj

@@ -17,8 +17,8 @@
 (expect
   {:status 404
    :body "Not found."}
-  (try (client/post (http/build-url "notify/db/100" {}) {:accept :json
-                                                         :headers {"X-METABASE-APIKEY" "test-api-key"}})
+  (try (client/post ((resolve 'metabase.http-client/build-url) "notify/db/100" {}) {:accept :json
+                                                                                    :headers {"X-METABASE-APIKEY" "test-api-key"}})
        (catch clojure.lang.ExceptionInfo e
          (-> (.getData ^clojure.lang.ExceptionInfo e)
              (:object)

test/metabase/http_client.clj

@@ -111,7 +111,7 @@
     (catch Exception e
       (log/error "Failed to authenticate with email:" email "and password:" password ". Does user exist?"))))
 
-(defn build-url [url url-param-kwargs]
+(defn- build-url [url url-param-kwargs]
   {:pre [(string? url)
          (or (nil? url-param-kwargs)
              (map? url-param-kwargs))]}

test/metabase/test_setup.clj

 (ns metabase.test-setup
   "Functions that run before + after unit tests (setup DB, start web server, load test data)."
-  (:require [clojure.java.io :as io]
+  (:require (clojure.java [classpath :as classpath]
+                          [io :as io])
             [clojure.set :as set]
             [clojure.tools.logging :as log]
+            [clojure.tools.namespace.find :as ns-find]
             [expectations :refer :all]
             (metabase [core :as core]
                       [db :as db]
@@ -94,3 +96,52 @@
   []
   (log/info "Shutting down Metabase unit test runner")
   (core/stop-jetty))
+
+
+
+;; Check that we're on target for every public var in Metabase having a docstring
+;; This will abort unit tests if we don't hit our target
+(defn- expected-docstr-percentage-for-day-of-year
+  "Calculate the percentage of public vars we expect to have a docstring by the current date in time. This ranges from 80% for the end of January to 100% half way through the year."
+  ([]
+   (expected-docstr-percentage-for-day-of-year (u/date-extract :day-of-year)))
+  ([doy]
+   (let [start-day                  30
+         start-percent              80.0
+         target-doy-for-100-percent 180
+         remaining-percent          (- 100.0 start-percent)
+         remaining-days             (- target-doy-for-100-percent start-day)]
+     (Math/min (+ start-percent (* (/ remaining-percent remaining-days)
+                                   (- doy start-day)))
+               100.0))))
+
+(defn- does-metabase-need-more-dox? []
+  (let [symb->has-doc?      (into {} (for [ns          (ns-find/find-namespaces (classpath/classpath))
+                                           :let        [nm (try (str (ns-name ns))
+                                                                (catch Throwable _))]
+                                           :when       nm
+                                           :when       (re-find #"^metabase" nm)
+                                           :when       (not (re-find #"test" nm))
+                                           [symb varr] (ns-publics ns)]
+                                       {(symbol (str nm "/" symb)) (boolean (:doc (meta varr)))}))
+        vs                  (vals symb->has-doc?)
+        total               (count vs)
+        num-with-dox        (count (filter identity vs))
+        percentage          (float (* (/ num-with-dox total)
+                                      100.0))
+        expected-percentage (expected-docstr-percentage-for-day-of-year)
+        needs-more-dox?     (< percentage expected-percentage)]
+    (println (u/format-color (if needs-more-dox? 'red 'green)
+                 "%.1f%% of Metabase public vars have docstrings. (%d/%d) Expected for today: %.1f%%" percentage num-with-dox total expected-percentage))
+    (println (u/format-color 'cyan "Why don't you go write a docstr for %s?" (first (shuffle (for [[symb has-doc?] symb->has-doc?
+                                                                                                   :when           (not has-doc?)]
+                                                                                               symb)))))
+    needs-more-dox?))
+
+
+(defn- throw-if-metabase-doesnt-have-enough-docstrings!
+  {:expectations-options :before-run}
+  []
+  (when (does-metabase-need-more-dox?)
+    (println (u/format-color 'red "Metabase needs more docstrings! Go write some more (or make some vars ^:private) before proceeding."))
+    (System/exit -1)))