Skip to content

Moderate changes and improvements; please review and, if appropriate, merge. #2

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

pprince
Copy link

@pprince pprince commented Jan 20, 2014

I release my changes under the same permissive license as specified in the LICENSE file, and you may feel free to incorporate them while attributing the copyright to yourself.

JSYK, I am also looking at generalizing this, to support a .env file or .env/ directory inside $VIRTUAL_ENV/. If I do that and you don't otherwise object, I will attribute the copyright on that project to myself, and list your two projects as inspiration. This is in case I want to standardize on releasing my projects under a different permissive license.

Feel free to shoot me an email,
-Paul Prince [email protected]

Differences in behavior from upstream version:

  * Both the values, **and the set/unset** status, of
    all environmental variables will be preserved and
    restored properly on deactivating the virtualenv.

  * Also corrected an incorrect comment.
@pprince
Copy link
Author

pprince commented Jan 20, 2014

ppdevel branch has individual commits;
all have been squashed to the master branch as of now.

also, tested / working as of now.

@pprince
Copy link
Author

pprince commented Jan 20, 2014

** To enable for a given virtualenv, touch $VIRTUAL_ENV/.npm

** TODO: Update docs.

@rach
Copy link
Owner

rach commented Jan 21, 2014

Thanks for the PR.
I only see one issue just by looking at the code, I will need to test it.
The issue is that previous npm installed which was providing a bin was becoming available in the $PATH.
It doesn't seem the case anymore. I think that I'm missing the idea behind the change.
I'm very happy to include it we can keep the bin working. I will check at that later this week.
Thanks again for the PR.

@pprince
Copy link
Author

pprince commented Jan 21, 2014

Yeah, so, the way I use it is, if you install node modules normally, npm install grunt, they go into ./node_modules and bins go into
./node_modules/bin, and you're right they're not on the PATH. But, that's
the typical behavior for node modules, and if people want that bin in
their path, I think they should do it themselves.

Since I'm setting NPM_HOME=$VIRTUAL_ENV, when you do a global install, npm install -g grunt-cli, they will install to $VIRTUAL_ENV and bins will go
to $VIRTUAL_ENV/bin which is already on the path.

@pprince
Copy link
Author

pprince commented Jan 21, 2014

basically some things expect (grunt, I think, was the one) to not be
installed globally, and will complain if not installed locally. So I keep
the separation between global and local installs, global into virtualenv
and local into projectdir, and I install most things locally. When I have
something that installs a command I will want to run (like grunt-cli), I
install it with -g. This mirrors typical usage of NPM, so that the
workflow is the same under virtualenvwrapper just better :)

Thanks,
-Paul P.

On Mon, Jan 20, 2014 at 7:21 PM, Rach Belaid [email protected]:

Thanks for the PR.
I only see one issue just by looking at the code, I will need to test it.
The issue is that previous npm installed which was providing a bin was
becoming available in the $PATH.
It doesn't seem the case anymore. I think that I'm missing the idea behind
the change.
I'm very happy to include it we can keep the bin working. I will check at
that later this week.
Thanks again for the PR.


Reply to this email directly or view it on GitHubhttps://github.com//pull/2#issuecomment-32810337
.

@pprince
Copy link
Author

pprince commented Jan 12, 2016

Hrm... this never made it in, eh?

If you are interested in these changes still, I can clean them up and get them current for the current version of the software.

Just let me know :)

@pprince
Copy link
Author

pprince commented Jan 12, 2016

I think I'm fine now with doing what you suggested, and having global install be the default once inside a project, and put node_module's bin/ on the $PATH. Or I could just make it configurable.

I don't like the way the software currently effects all of my virtualenvwrapper projects, though, and would love to get my per-project .npm / .gem support into your mainline :)

@rach
Copy link
Owner

rach commented Jan 13, 2016

Hi Sorry, I had completely forgot about this PR. I'll have a look tonight.
I think that I did some changes not a long time ago to avoid to install npm globally per default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants