From ffda98e5826e17cc31c890510c851da2dcfe9415 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cam=20Sa=C3=BCl?= <cammsaul@gmail.com>
Date: Tue, 1 Mar 2016 16:26:39 -0800
Subject: [PATCH] Fix linters :cry:
---
bin/reflection-linter | 4 +++-
circle.yml | 4 ++--
docs/developers-guide.md | 9 +------
project.clj | 11 +++++----
src/metabase/driver/druid/query_processor.clj | 21 +++++++++-------
.../driver/generic_sql/query_processor.clj | 24 ++++++++++++++-----
src/metabase/events/activity_feed.clj | 4 +++-
src/metabase/events/dependencies.clj | 6 +++--
src/metabase/events/last_login.clj | 4 +++-
src/metabase/events/notifications.clj | 4 +++-
src/metabase/events/revision.clj | 4 +++-
src/metabase/events/sync_database.clj | 4 +++-
src/metabase/events/view_log.clj | 6 +++--
src/metabase/task/send_pulses.clj | 4 +++-
src/metabase/task/sync_databases.clj | 4 +++-
15 files changed, 72 insertions(+), 41 deletions(-)
diff --git a/bin/reflection-linter b/bin/reflection-linter
index 5de9aad925c..c661f4b91ef 100755
--- a/bin/reflection-linter
+++ b/bin/reflection-linter
@@ -1,6 +1,8 @@
#!/usr/bin/env bash
-warnings=`lein check 2>&1 | grep Reflection | grep metabase`
+echo -e "\e[1;34mChecking for reflection warnings. This may take a few minutes, so sit tight...\e[0m"
+
+warnings=`lein check-reflection-warnings 2>&1 | grep Reflection | grep metabase | uniq`
if [ ! -z "$warnings" ]; then
echo -e "\e[1;31mYour code has cased introduced some reflection warnings.\e[0m 😞"
diff --git a/circle.yml b/circle.yml
index a4471f099e9..dd4de332ee9 100644
--- a/circle.yml
+++ b/circle.yml
@@ -26,10 +26,10 @@ test:
# 1) runs unit tests w/ Postgres local DB. Runs against H2, SQL Server
# 2) runs unit tests w/ MySQL local DB. Runs against H2, Postgres, SQLite
# 3) runs unit tests w/ H2 local DB. Runs against H2, Redshift, Druid
- # 4) runs docstring-checker linter && Eastwood linter & Bikeshed linter && ./bin/reflection-linter
+ # 4) runs Eastwood linter, Bikeshed linter, docstring-checker & ./bin/reflection-linter
# 5) runs JS linter + JS test
# 6) runs lein uberjar. (We don't run bin/build because we're not really concerned about `npm install` (etc) in this test, which runs elsewhere)
- - case $CIRCLE_NODE_INDEX in 0) ENGINES=h2,mongo,mysql,bigquery lein test ;; 1) ENGINES=h2,sqlserver MB_DB_TYPE=postgres MB_DB_DBNAME=circle_test MB_DB_PORT=5432 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 2) ENGINES=h2,postgres,sqlite MB_DB_TYPE=mysql MB_DB_DBNAME=circle_test MB_DB_PORT=3306 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 3) ENGINES=h2,redshift,druid lein test ;; 4) lein eastwood 2>&1 | grep -v Reflection && lein bikeshed 2>&1 | grep -v Reflection && ./bin/reflection-linter && lein docstring-checker ;; 5) npm install && npm run lint && npm run build && npm run test ;; 6) lein uberjar ;; esac:
+ - case $CIRCLE_NODE_INDEX in 0) ENGINES=h2,mongo,mysql,bigquery lein test ;; 1) ENGINES=h2,sqlserver MB_DB_TYPE=postgres MB_DB_DBNAME=circle_test MB_DB_PORT=5432 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 2) ENGINES=h2,postgres,sqlite MB_DB_TYPE=mysql MB_DB_DBNAME=circle_test MB_DB_PORT=3306 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 3) ENGINES=h2,redshift,druid lein test ;; 4) lein eastwood && lein bikeshed && lein docstring-checker && ./bin/reflection-linter ;; 5) npm install && npm run lint && npm run build && npm run test ;; 6) lein uberjar ;; esac:
parallel: true
deployment:
master:
diff --git a/docs/developers-guide.md b/docs/developers-guide.md
index 2d00f4b5aa3..cd7af764339 100644
--- a/docs/developers-guide.md
+++ b/docs/developers-guide.md
@@ -142,7 +142,7 @@ when testing since they are impossible to run locally (such as Redshift and Bigq
Run the linters:
- lein eastwood && lein bikeshed
+ lein eastwood && lein bikeshed && lein docstring-checker && ./bin/reflection-linter
#### Developing with Emacs
@@ -159,13 +159,6 @@ You'll probably want to tell Emacs to store customizations in a different file.
(load-file custom-file))
```
-#### Checking for Out-of-Date Dependencies
-
- lein ancient # list all out-of-date dependencies
- lein ancient latest lein-ring # list latest version of artifact lein-ring
-
-Will give you a list of out-of-date dependencies.
-
## Documentation
#### Instant Cheatsheet
diff --git a/project.clj b/project.clj
index 24b01a93b7a..8eb7dbb9984 100644
--- a/project.clj
+++ b/project.clj
@@ -5,7 +5,8 @@
:description "Metabase Community Edition"
:url "http://metabase.com/"
:min-lein-version "2.5.0"
- :aliases {"bikeshed" ["with-profile" "+bikeshed" "bikeshed" "--max-line-length" "240"]
+ :aliases {"bikeshed" ["bikeshed" "--max-line-length" "240"]
+ "check-reflection-warnings" ["with-profile" "+reflection-warnings" "check"]
"test" ["with-profile" "+expectations" "expectations"]
"generate-sample-dataset" ["with-profile" "+generate-sample-dataset" "run"]}
:dependencies [[org.clojure/clojure "1.8.0"]
@@ -76,7 +77,8 @@
:ring {:handler metabase.core/app
:init metabase.core/init!
:destroy metabase.core/destroy}
- :eastwood {:exclude-namespaces [:test-paths]
+ :eastwood {:exclude-namespaces [:test-paths
+ metabase.driver.generic-sql] ; ISQLDriver causes Eastwood to fail. Skip this ns until issue is fixed: https://github.com/jonase/eastwood/issues/191
:add-linters [:unused-private-vars]
:exclude-linters [:constant-test ; korma macros generate some forms with if statements that are always logically true or false
:suspicious-expression ; core.match macros generate some forms like (and expr) which is "suspicious"
@@ -88,11 +90,9 @@
:profiles {:dev {:dependencies [[org.clojure/tools.nrepl "0.2.12"] ; REPL <3
[expectations "2.1.3"] ; unit tests
[ring/ring-mock "0.3.0"]]
- :plugins [[docstring-checker "1.0.0"] ; Check that all public vars have docstrings
+ :plugins [[docstring-checker "1.0.0"] ; Check that all public vars have docstrings. Run with 'lein docstring-checker'
[jonase/eastwood "0.2.3"
:exclusions [org.clojure/clojure]] ; Linting
- [lein-ancient "0.6.8" ; Check project for outdated dependencies + plugins w/ 'lein ancient'
- :exclusions [org.clojure/clojure]]
[lein-bikeshed "0.3.0"] ; Linting
[lein-expectations "0.0.8"] ; run unit tests with 'lein expectations'
[lein-instant-cheatsheet "2.2.1" ; use awesome instant cheatsheet created by yours truly w/ 'lein instant-cheatsheet'
@@ -104,6 +104,7 @@
"-Xmx2048m" ; hard limit of 2GB so we stop hitting the 4GB container limit on CircleCI
"-XX:+CMSClassUnloadingEnabled" ; let Clojure's dynamically generated temporary classes be GC'ed from PermGen
"-XX:+UseConcMarkSweepGC"]} ; Concurrent Mark Sweep GC needs to be used for Class Unloading (above)
+ :reflection-warnings {:global-vars {*warn-on-reflection* true}} ; run `lein check-reflection-warnings` to check for reflection warnings
:expectations {:injections [(require 'metabase.test-setup)]
:resource-paths ["test_resources"]
:env {:mb-test-setting-1 "ABCDEFG"
diff --git a/src/metabase/driver/druid/query_processor.clj b/src/metabase/driver/druid/query_processor.clj
index 8fa74b67db5..5f7f4391c3a 100644
--- a/src/metabase/driver/druid/query_processor.clj
+++ b/src/metabase/driver/druid/query_processor.clj
@@ -97,7 +97,7 @@
:fnAggregate "function(current, x) { return current + (parseFloat(x) || 0); }"
:fnCombine "function(x, y) { return x + y; }"}))
-(defn- ag:filtered [filter aggregator] {:type :filtered, :filter filter, :aggregator aggregator})
+(defn- ag:filtered [filtr aggregator] {:type :filtered, :filter filtr, :aggregator aggregator})
(defn- ag:count
([output-name] {:type :count, :name output-name})
@@ -208,11 +208,11 @@
;;; ### handle-filter
-(defn- filter:not [filter]
- {:pre [filter]}
- (if (= (:type filter) :not) ; it looks like "two nots don't make an identity" with druid
- (:field filter)
- {:type :not, :field filter}))
+(defn- filter:not [filtr]
+ {:pre [filtr]}
+ (if (= (:type filtr) :not) ; it looks like "two nots don't make an identity" with druid
+ (:field filtr)
+ {:type :not, :field filtr}))
(defn- filter:= [field value]
{:type :selector
@@ -282,8 +282,13 @@
nil (parse-filter-subclause:filter clause)))
-(defn- make-intervals [min max & more]
- (vec (concat [(str (or (->rvalue min) -5000) "/" (or (->rvalue max) 5000))]
+(defn- make-intervals
+ "Make a value for the `:intervals` in a Druid query.
+
+ ;; Return results in 2012 or 2015
+ (make-intervals 2012 2013 2015 2016) -> [\"2012/2013\" \"2015/2016\"]"
+ [interval-min interval-max & more]
+ (vec (concat [(str (or (->rvalue interval-min) -5000) "/" (or (->rvalue interval-max) 5000))]
(when (seq more)
(apply make-intervals more)))))
diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj
index e8eb3fb0830..70e338332bf 100644
--- a/src/metabase/driver/generic_sql/query_processor.clj
+++ b/src/metabase/driver/generic_sql/query_processor.clj
@@ -122,7 +122,9 @@
:when (not (contains? (set fields-fields) field))]
(as (formatted field) field)))))
-(defn- apply-fields [_ korma-form {fields :fields}]
+(defn apply-fields
+ "Apply a `fields` clause to KORMA-FORM. Default implementation of `apply-fields` for SQL drivers."
+ [_ korma-form {fields :fields}]
(apply k/fields korma-form (for [field fields]
(as (formatted field) field))))
@@ -150,10 +152,14 @@
:not (kfns/pred-not (kengine/pred-map (filter-subclause->predicate subclause)))
nil (filter-subclause->predicate clause)))
-(defn- apply-filter [_ korma-form {clause :filter}]
+(defn apply-filter
+ "Apply a `filter` clause to KORMA-FORM. Default implementation of `apply-filter` for SQL drivers."
+ [_ korma-form {clause :filter}]
(k/where korma-form (filter-clause->predicate clause)))
-(defn- apply-join-tables [_ korma-form {join-tables :join-tables, {source-table-name :name, source-schema :schema} :source-table}]
+(defn apply-join-tables
+ "Apply expanded query `join-tables` clause to KORMA-FORM. Default implementation of `apply-join-tables` for SQL drivers."
+ [_ korma-form {join-tables :join-tables, {source-table-name :name, source-schema :schema} :source-table}]
(loop [korma-form korma-form, [{:keys [table-name pk-field source-field schema]} & more] join-tables]
(let [table-name (if (seq schema)
(str schema \. table-name)
@@ -168,10 +174,14 @@
(recur korma-form more)
korma-form))))
-(defn- apply-limit [_ korma-form {value :limit}]
+(defn apply-limit
+ "Apply `limit` clause to KORMA-FORM. Default implementation of `apply-limit` for SQL drivers."
+ [_ korma-form {value :limit}]
(k/limit korma-form value))
-(defn- apply-order-by [_ korma-form {subclauses :order-by}]
+(defn apply-order-by
+ "Apply `order-by` clause to KORMA-FORM. Default implementation of `apply-order-by` for SQL drivers."
+ [_ korma-form {subclauses :order-by}]
(loop [korma-form korma-form, [{:keys [field direction]} & more] subclauses]
(let [korma-form (k/order korma-form (formatted field) (case direction
:ascending :ASC
@@ -180,7 +190,9 @@
(recur korma-form more)
korma-form))))
-(defn- apply-page [_ korma-form {{:keys [items page]} :page}]
+(defn apply-page
+ "Apply `page` clause to KORMA-FORM. Default implementation of `apply-page` for SQL drivers."
+ [_ korma-form {{:keys [items page]} :page}]
(-> korma-form
(k/limit items)
(k/offset (* items (dec page)))))
diff --git a/src/metabase/events/activity_feed.clj b/src/metabase/events/activity_feed.clj
index 38fb17d11f1..e4f592e0690 100644
--- a/src/metabase/events/activity_feed.clj
+++ b/src/metabase/events/activity_feed.clj
@@ -149,5 +149,7 @@
;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
-(defn- events-init []
+(defn events-init
+ "Automatically called during startup; start the events listener for the activity feed."
+ []
(events/start-event-listener activity-feed-topics activity-feed-channel process-activity-event))
diff --git a/src/metabase/events/dependencies.clj b/src/metabase/events/dependencies.clj
index 5cbaa64cc06..addad4a8c28 100644
--- a/src/metabase/events/dependencies.clj
+++ b/src/metabase/events/dependencies.clj
@@ -7,7 +7,7 @@
[metric :refer [Metric]])))
-(def ^:const dependencies-topics
+(def ^:private ^:const dependencies-topics
"The `Set` of event topics which are subscribed to for use in dependencies tracking."
#{:card-create
:card-update
@@ -47,5 +47,7 @@
;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
-(defn- events-init []
+(defn events-init
+ "Automatically called during startup; start the events listener for dependencies topics."
+ []
(events/start-event-listener dependencies-topics dependencies-channel process-dependencies-event))
diff --git a/src/metabase/events/last_login.clj b/src/metabase/events/last_login.clj
index f7362b5b8bd..fe9d1b059cb 100644
--- a/src/metabase/events/last_login.clj
+++ b/src/metabase/events/last_login.clj
@@ -37,5 +37,7 @@
;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
-(defn- events-init []
+(defn events-init
+ "Automatically called during startup; start the events listener for last login events."
+ []
(events/start-event-listener last-login-topics last-login-channel process-last-login-event))
diff --git a/src/metabase/events/notifications.clj b/src/metabase/events/notifications.clj
index 940c60e5451..03c2c1090ac 100644
--- a/src/metabase/events/notifications.clj
+++ b/src/metabase/events/notifications.clj
@@ -92,5 +92,7 @@
;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
-(defn- events-init []
+(defn events-init
+ "Automatically called during startup; start event listener for notifications events."
+ []
(events/start-event-listener notifications-topics notifications-channel process-notifications-event))
diff --git a/src/metabase/events/revision.clj b/src/metabase/events/revision.clj
index 8f2fdd97bfa..d77c94dbf6d 100644
--- a/src/metabase/events/revision.clj
+++ b/src/metabase/events/revision.clj
@@ -77,5 +77,7 @@
;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
-(defn- events-init []
+(defn events-init
+ "Automatically called during startup; start event listener for revision events."
+ []
(events/start-event-listener revisions-topics revisions-channel process-revision-event))
diff --git a/src/metabase/events/sync_database.clj b/src/metabase/events/sync_database.clj
index 4bdde22a292..59a0d7b32bc 100644
--- a/src/metabase/events/sync_database.clj
+++ b/src/metabase/events/sync_database.clj
@@ -40,5 +40,7 @@
;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
-(defn- events-init []
+(defn events-init
+ "Automatically called during startup; start event listener for database sync events."
+ []
(events/start-event-listener sync-database-topics sync-database-channel process-sync-database-event))
diff --git a/src/metabase/events/view_log.clj b/src/metabase/events/view_log.clj
index 1fbde879c10..3dd553dd874 100644
--- a/src/metabase/events/view_log.clj
+++ b/src/metabase/events/view_log.clj
@@ -6,7 +6,7 @@
[metabase.models.view-log :refer [ViewLog]]))
-(def ^:const view-counts-topics
+(def ^:private ^:const view-counts-topics
"The `Set` of event topics which we subscribe to for view counting."
#{:card-create
:card-read
@@ -46,5 +46,7 @@
;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
-(defn- events-init []
+(defn events-init
+ "Automatically called during startup; start the events listener for view events."
+ []
(events/start-event-listener view-counts-topics view-counts-channel process-view-count-event))
diff --git a/src/metabase/task/send_pulses.clj b/src/metabase/task/send_pulses.clj
index f5c6a8603e7..0dbdc65cc68 100644
--- a/src/metabase/task/send_pulses.clj
+++ b/src/metabase/task/send_pulses.clj
@@ -64,7 +64,9 @@
curr-monthweek (monthweek now)]
(send-pulses! curr-hour curr-weekday curr-monthday curr-monthweek)))
-(defn- task-init []
+(defn task-init
+ "Automatically called during startup; start the job for sending pulses."
+ []
;; build our job
(reset! send-pulses-job (jobs/build
(jobs/of-type SendPulses)
diff --git a/src/metabase/task/sync_databases.clj b/src/metabase/task/sync_databases.clj
index 532a5e24c1a..162c43e6750 100644
--- a/src/metabase/task/sync_databases.clj
+++ b/src/metabase/task/sync_databases.clj
@@ -26,7 +26,9 @@
(catch Throwable e
(log/error "Error syncing database: " (:id database) e))))))
-(defn- task-init []
+(defn task-init
+ "Automatically called during startup; start the job for syncing databases."
+ []
;; build our job
(reset! sync-databases-job (jobs/build
(jobs/of-type SyncDatabases)
--
GitLab