Skip to content
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

[fix] Allow developing without nvm/root #2223

Closed
wants to merge 1 commit into from
Closed

[fix] Allow developing without nvm/root #2223

wants to merge 1 commit into from

Conversation

insidewhy
Copy link

See my comments in #2086

I assume that some changes happened to lerna which broke the env.js script. I assume the people developing on enzyme recently are all doing so with root access. Naughty!

Anyway, this fixes it. Now I can develop.

env.js Outdated
run('npm', 'i', '--no-save', ...installs),
Promise.all(
rmrfs.map(s => primraf(s))
).then(() => run('npm', 'i', '--no-save', ...installs)),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous npm i in this script readds the node_modules/npm link... so it has to be removed before this npm i --no-save otherwise the same problem just happens again.

@ljharb
Copy link
Member

ljharb commented Aug 21, 2019

On the contrary; we’re all (me) using nvm, so no root access is required. I’d suggest that approach.

@insidewhy
Copy link
Author

@ljharb I'd suggest just merging this so that people don't have to use nvm if they don't want to. I'm just about to push an improvement though, so hang on a few minutes.

@insidewhy
Copy link
Author

Okay all done now.

@insidewhy insidewhy changed the title [fix] Allow developing without root access [fix] Allow developing without nvm/root Aug 21, 2019
env.js Show resolved Hide resolved
@insidewhy
Copy link
Author

Hm... travis is failing on stuff not related to this PR at all. mega-shrug. Also noticed the 16 version tests are broke when I run locally, and also in another recent travis build unrelated to my stuff.

@ljharb
Copy link
Member

ljharb commented Aug 23, 2019

I believe the 16.9 release broke some of our tests; I'll rebase and merge this once master is passing again.

@insidewhy
Copy link
Author

@ljharb Cool thanks. I'll resume work on the hooks support once that is done. As of now, I can't get the tests to pass reliably even once I revert all my changes and use the pure github master. I tried them in the node:10, node:11 and node:12 containers so there shouldn't be anything about my system interfering here.

@ljharb
Copy link
Member

ljharb commented Aug 31, 2019

@ohjames the tests are broken on master with the release of react 16.9; all work on enzyme is blocked on fixing that. I'll merge this as soon as I can get master passing again.

@insidewhy insidewhy closed this Aug 31, 2019
@ljharb ljharb reopened this Aug 31, 2019
@ljharb ljharb self-requested a review September 28, 2019 04:35
@ljharb
Copy link
Member

ljharb commented Sep 28, 2019

@ohjames i've rebased this, but the tests are failing legitimately.

@insidewhy insidewhy closed this Oct 23, 2019
@ljharb ljharb reopened this Oct 23, 2019
@ljharb
Copy link
Member

ljharb commented Oct 23, 2019

@insidewhy are you no longer interested in this PR?

@insidewhy insidewhy closed this Jun 26, 2020
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