search

sattvik / leinjacker

A library for Leiningen plug-in authors.
42 stars 11 forks source link

Fixed bug in hook-eval-in-project and added project map merging #6

Closed dgrnbrg closed 11 years ago

dgrnbrg commented 11 years ago

I found a bug in the way I was passing arguments through the eval-in-project, so that it would fail if you depended on the init arguments being passed successfully. (previously, I had no such requirement). This is now fixed.

Secondly, I added a leiningen-agnostic way to merge project maps. Unfortunately, in lein2, you cannot simple conj a new dependency onto the project map, because when a task gets called, leiningen will unmerge and remerge the profiles, obliterating the change the the :dependencies. In order to support merging in new dependencies and other keys to the project map, you must use add-profile and merge-profile. On the other hand, lein1 has no knowledge of any of this, and provides no way to recursively sanely merge projects. I've added a function that provides recursive project merging for lein1 and lein2. This is useful for higher-order tasks, like ztellman/sleight and dgrnbrg/guzheng, which I'm updating with this new functionality so that they'll be cross-leiningen compatible.

dgrnbrg commented 11 years ago

I've just added a test to confirm that init and the body are passed into eval-in-project correctly.

I'm not sure if there's a reasonable way to test the profile-merging--it's a simplified copy of Leiningen2's code (removing metadata checks related to profiles) for the Lein1 impl, and the Lein2 impl is a copy of some code already in use in guzheng and sleight. To properly test this, it would require the creation of multiple higher-order test plugins to ensure that data wasn't removed; however, that is a lot of work for relatively simple code. What do you think?

dgrnbrg commented 11 years ago

Ping! I am just wondering whether this could be merged, so that I can propagate the changes to sleight & guzheng downstream.

Thanks!

sattvik commented 11 years ago

Hello, David,

I'll look into it this evening.

sattvik commented 11 years ago

I apologise for the delay. I have merged in the fix and release leinjacker-0.3.1 with your fix.

sattvik commented 11 years ago

Make that leinjacker-0.3.2. It turns out core.contracts is not a drop-in replacement for trammel. I should have tested that better.

dgrnbrg commented 11 years ago

Thank you!

On Sun, Oct 14, 2012 at 3:15 PM, Daniel Solano Gómez < notifications@github.com> wrote:

Make that leinjacker-0.3.2. It turns out core.contracts is not a drop-in replacement for trammel. I should have tested that better.

— Reply to this email directly or view it on GitHubhttps://github.com/sattvik/leinjacker/pull/6#issuecomment-9424590.