Closed tobias closed 12 years ago
Thanks! I haven't finished looking through all of it, but it looks good. I'll let you know if I have any questions.
@sattvik - have you had a chance to look at this further?
I apologise for taking so long to take a review this. From what i have seen of the code it looks good. I will take some time later today to see that all the tests run. If so, then I'll pull it in.
An update to let you know what's going on. I started trying to run the tests, and I run into a few problems:
java.lang.NoSuchMethodError: clojure.lang.RT.keyword(Ljava/lang/String;Ljava/lang/String;)Lclojure/lang/Keyword;
when running the lein1 tests.It's gotten pretty late for me, so I'll have to look into this third issue deeper tomorrow.
Sorry you are having issues with the tests - I just checked again and they pass for me here. I went ahead and pushed a change that adds lein-midje as a dev dependency for the test project. What OS are you running the tests on? I've only tested on MacOS, and I wonder if that makes a difference for the first issue?
I have seen the third issue before on another project - I believe it was caused by having code on the classpath that was aot compiled with a different version of clojure than is currently being used. But that shouldn't be the case here, since there is no aot compilation.
I'm happy to take a further look at this next week, and can try to recreate your issues then if you don't have a chance to chase them down over the weekend.
Well, I figured out what the problem was. The 'LEIN_JAVA_CMD' environment variable from Lein 2 was messing up the Lein1 runtime. Removing it from the environment fixed both issues 1 and 3 above.
Thanks for all your help. Again, I apologise for not getting to this sooner.
I finally found some time today to work on this. This patch includes the following changes:
eval-in-project
ns is now namedeval
, and includes ash
functionutils
ns now haslein-home
,lein-generation
,read-lein-project
, andtry-resolve-any
, which basically mapstry-resolve
to a collection of symbols, returning the first success.test-project/
). Its tests are run via the poorly namedtest/leinjacker/multiple_leins_runner.clj
.eval
andutils
now have basic tests withintest-project/
try-resolve
andtry-resolve-any
have tests intest/leinjacker/utils_test.clj
Running the tests currently is slow, since the mechanism for running the tests in the sub-project does the following:
lein install
to install leinjackerlein
,lein1
, andlein2
in an effort to find the proper executable name for each version (this is cached to a file upon success to avoid paying for this every time)lein deps
intest-project/
for lein1 to get the leinjacker jar in the proper locationlein{2} midje
intest-project/
for each versionSteps 1 and 3 could be avoided if I could come up with a better way to get leinjacker onto the sub-project's classpath, but I haven't yet found a way to do that that also gets leinjacker's own dependencies loaded properly.
Any feedback is welcome!