Skip to content

Conversation

johnhaley81
Copy link
Collaborator

@johnhaley81 johnhaley81 commented Jun 14, 2016

We were doing our own highly custom (read: fragile) life cycle scripting for installing NodeGit.

This should make it more in line with npm standards.

We were doing our own highly custom (read: fragile) life cycle scripting for installing NodeGit.

This should make it more in line with npm standards
Update to latest lodash
@johnhaley81 johnhaley81 force-pushed the lifecycle-fixes branch 2 times, most recently from b0f57f5 to c9c8cf8 Compare June 14, 2016 22:01
Also adds console output to let the user know a bit more 
about what's currently being done during install.
@johnhaley81
Copy link
Collaborator Author

@tbranyen @maxkorp we're passing CI. Can you guys give some 👀 on this?

@tbranyen
Copy link
Member

I'll test it tonight 😍

@maxkorp
Copy link
Collaborator

maxkorp commented Jun 23, 2016

Talked with john about this in person a moment ago, but putting here for records.

We can't clean the source afterwards.

The standard workflow for electron/nwjs is to npm install normally, then run a rebuild with the appropriate headers. This works with all other native modules i've seen, and currently works with nodegit, but cleaning the src/includes out would break that. We have to be able to run npm run rebuild after install has finished.

Otherwise, gonna review it up right now.

/build/
/examples/
/generate/
/guides/
/lib/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why ignore lib? all of our js is in there?

Copy link
Collaborator Author

@johnhaley81 johnhaley81 Jun 23, 2016

Choose a reason for hiding this comment

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

lib should already be 100% built and in dist.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes complete and total sense and I should have realized that, but sometimes I am do the forget.

@maxkorp
Copy link
Collaborator

maxkorp commented Jun 23, 2016

This should be good now :)

"lodash": "~3.10.1",
"mocha": "~2.3.4",
"nan": "^2.2.0",
"node-gyp": "~3.0.3",
Copy link
Collaborator

@maxkorp maxkorp Jun 23, 2016

Choose a reason for hiding this comment

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

The gyp exclusions make me SO GODDAMNED HAPPY.

@tbranyen
Copy link
Member

I'm so far removed from the lifecycle hooks code lately, but this looks v. good, nice work!

@maxkorp
Copy link
Collaborator

maxkorp commented Jun 23, 2016

@tbranyen There are a lot of improvements in npm3, but there are some definite regressions, and it doesn't fix the fact that npm2 is bundled with node 4 which is LTS :/

By excluding having to generate the code in our bundles, we just have to configure libssh2 if needed, ensure our deps are all installed when we run node-pre-gyp (this is not guaranteed properly in npm2, but it is in npm3 and trying to insure it ourself anyways actually breaks in npm3). We fall back to the default node-gyp now, since all that confusion was from A) node 0.8 and then came back in iojs 1/2.

A lot has changed, but overall the process is easier to follow now in npm3.

@johnhaley81 johnhaley81 merged commit 5f0fe1c into master Jun 23, 2016
@johnhaley81 johnhaley81 deleted the lifecycle-fixes branch June 23, 2016 18:59
@tbranyen
Copy link
Member

🎉

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.

3 participants