nervous-systems / serverless-cljs-plugin

Serverless plugin for Clojurescript deployment w/ cljs-lambda
The Unlicense
75 stars 10 forks source link

Fix lumo build configuration merge, close #11 #12

Closed arichiardi closed 7 years ago

moea commented 7 years ago

@arichiardi To do this reliably, you have to do it recursively.

(defn- merge-most [& maps]
  (apply merge-with
         (fn [x y]
           (cond (map?    x)  (merge-most x y)
                 (seqable? x) (into x y)
                 :else        y))
         maps))

However, because there is no support for ^:replace, etc. I don't think the sequence merging behaviour is helpful. I prefer this:

(defn- merge-maps [& maps]
  (apply merge-with
         (fn [x y]
           (if (map? x)
             (merge-maps x y)
             y))
         maps))
arichiardi commented 7 years ago

Of course, but I did not want to overcomplicate things.

We have one level of sub-maps in our edn only.

Your implementation actually takes the latter for everything that is not a map. I have heard people complaining that lein does not merge source-paths in project clj and in :cljsbuild... I guess it is a matter of taste and I am fine as long as the semantic is correct and clear

Will change with your implementation.

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

arichiardi commented 7 years ago

Fixed that, just drop the apply because we don't really need to merge multiple maps here anyways. Hope you don't mind :wink: