weavejester / lein-ring

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

Issue #168 add-if-dependencies ignores transitive dependencies and th… #169

Closed ekimber closed 9 years ago

ekimber commented 9 years ago

Pull request to fix issue #168

add-if-dependencies ignores transitive dependencies and therefore may unexpectedly downgrade the project version of ring-servlet.

MichaelBlume commented 9 years ago

This is going to break a lot of builds suddenly. Is there a way to make add-if-missing check for transitive dependencies? Hell, if absolutely necessary we could eval a call to require within the project and add the dependency if the call fails.

ekimber commented 9 years ago

I had a quick look at the add-if-missing function and as I recall it didn't look like it could be fixed. One other alternative would simply be to keep up with the latest version of ring servlet.

Also, yes I was aware that this would break builds but there is a similar issue with the ring library itself, which no longer includes the servlet api dependency, see here https://github.com/ring-clojure/ring near the top there's the 'Upgrade Notice'.

MichaelBlume commented 9 years ago

Ok, so I can see three ways to do this, one is the one you're suggesting, which puts an extra requirement on the app author, one is to just keep the ring-servlet dependency in lein-ring up to date, and one is to try requiring a namespace from ring-servlet in the app and then adding the dependency if that throws.

@weavejester , any thoughts?

weavejester commented 9 years ago

We should just use the latest dependency. Ring 1.4.0 is backward compatible with Ring 1.2.1 so it should be fine to update the dependency.

MichaelBlume commented 9 years ago

Done, and released =)

On Sun, Sep 27, 2015 at 7:31 AM James Reeves notifications@github.com wrote:

We should just use the latest dependency. Ring 1.4.0 is backward compatible with Ring 1.2.1 so it should be fine to update the dependency.

— Reply to this email directly or view it on GitHub https://github.com/weavejester/lein-ring/pull/169#issuecomment-143563058 .

ekimber commented 9 years ago

OK I'll close this PR