-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle :repositories containing {:username :env/foo, :password :env/bar} #13
Conversation
…nv/foo :password :env/bar}
(defn resolve-repositories [project] | ||
(->> (:repositories project) | ||
(map (fn [[name repo]] | ||
[name (leiningen.core.user/resolve-credentials repo)])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd prefer if we explicitly required leningen.core.user, instead of just assuming it was loaded. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iirc, the potential absence of the namespace will be noticed at compile time anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, you are probably right. I suppose it doesn't matter then. I could still make the argument from a readability perspective. Typically one would determine what libraries a namespace depends on by simply looking at the usage of the ns
macro at the top of the file.
I'm fine with it as is though.
Bump! Having the same issue here. |
@RyanMcG you were right that resolve-repos takes the repositories, and not a project. The patch worked for me though, because it had the side-effect of replacing :repositories with the empty map. This avoided the original bug because maven repos aren't necessary for resolving npm packages. I guess the larger question for me is, why does npm.deps spend so much effort with mvn repos? |
@@ -1,4 +1,4 @@ | |||
(defproject lein-npm "0.4.0" | |||
(defproject arohner/lein-npm "0.4.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't modify project.clj unless you need to. Definitely don't change the project name or version number.
Some recursion is done looking at your dependencies lein-npm dependencies. At least I believe that is what you are referring to. |
Is there anything I can do to help get this merged? This bit me in the butt today. |
@danielcompton I'd been hoping that @arohner would come back so I could merge his stuff. I'll try and get this merged in when I get home today if @arohner doesn't beat me to it. |
This will be in 0.5.1. The version bump is already done, but I have been unable to deploy because I don't have access to my public key at the moment (this is very silly and I apologize). I'll make sure I figure how to sync it better across my devices. |
This calls leiningen.core.user/resolve-credentials on repositories. It fixes a bug
where if you have in your project.clj
you'll get this stacktrace when running
lein npm install