search

weavejester / lein-ring

Ring plugin for Leiningen
Eclipse Public License 1.0
500 stars 100 forks source link

Fixing issue #126 with Leiningen 2.5.0 due to auto-cleaning of targets #127

Closed tranchis closed 10 years ago

tranchis commented 10 years ago

Fixing issue #126 with Leiningen 2.5.0 due to auto-cleaning of targets being the default behaviour and thus wiping the auto-compiled main class before packing.

With this fix the auto-clean, if not set to false in project.clj, is made before compile-main.

tranchis commented 10 years ago

Updated. I have separated the modification function and put it into util.clj, to make uberjar.clj a bit cleaner, which can also be useful if it has to be used from somewhere else (jar.clj?).

weavejester commented 10 years ago

Since the function is specifically for the uberjar profile, could you put it back into the uberjar namespace as a private function? I don't think it'll be useful outside the uberjar command, and if it is, we can move it out then rather than try and anticipate what we'll need in future.

tranchis commented 10 years ago

Done!

weavejester commented 10 years ago

Thanks! Could you also use the vary-meta function? So instead of:

(with-meta project
    (assoc-in (meta project) 
              [:profiles :uberjar :auto-clean] false))

Use the more concise:

(vary-meta project assoc-in [:profiles :uberjar :auto-clean] false))

Can you also make sure that the commits are squashed into one, and that the commit summary (i.e. the first line) doesn't exceed 70 characters to avoid it being truncated. It would also be helpful if the commit messages were written in the imperative voice, e.g. "fix" instead of "fixing" or "fixed".

tranchis commented 10 years ago

I think I've done it the right way, but I'm not sure... It's the first time I'm using the rebase+squash feature :)

weavejester commented 10 years ago

Almost right. Just clear out the parts of the commit message that are no longer relevant (you should be able to use git commit --amend). You can probably just strip it down to the first line.

tranchis commented 10 years ago

I've rebased everything to keep only one commit, I think it's cleaner now.

weavejester commented 10 years ago

Merged and released as version 0.8.12.

I've also recently adopted Leiningen's commit model for this repository (which I should really mention in the README), so any successfully merged PR results in commit access to the repo.