Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

pull in parts of mafintosh/leveldown-prebuilt#master #188

Merged
merged 23 commits into from
Jun 1, 2015
Merged

pull in parts of mafintosh/leveldown-prebuilt#master #188

merged 23 commits into from
Jun 1, 2015

Conversation

ralphtheninja
Copy link
Member

No description provided.

@ralphtheninja ralphtheninja mentioned this pull request May 30, 2015
@ralphtheninja
Copy link
Member Author

Seems to be some problems on travis for iojs versions. Not sure what's going on here. /cc @mafintosh

@ralphtheninja
Copy link
Member Author

This should be changed to something more suitable:

leveldown-prebuilt

@mafintosh
Copy link
Member

Yes we should probably just remove the "bump version" commits + change the host to this repo

@ralphtheninja
Copy link
Member Author

@mafintosh Hmm yes. I'm wondering what's the best way to do that. Perhaps I should just take your master branch and git reset --mixed <suitable_sha1> and push up a new branch.

@ralphtheninja
Copy link
Member Author

So this is kind of tricky to rewrite since the master branch of leveldown has been merged twice into your branch. I think I prefer just doing a merge from Level/leveldown#master into mafintosh/leveldown-prebuilt#master and push up a new branch. The trade off is that commit history is a bit dirty. I'll play around a bit first to see if I can make it work.

@ralphtheninja
Copy link
Member Author

@mafintosh I did as follows.

I created a new branch based on commit 541a065:

prebuilt-commit

Then merged latest leveldown#master into that branch.

Essentially undoing:

undo1

and

undo2

@ralphtheninja ralphtheninja changed the title node-pre-gyp merged in from mafintosh/leveldown-prebuilt/master pull in parts of mafintosh/leveldown-prebuilt#master May 30, 2015
},
"license": "MIT",
"gypfile": true
"gypfile": true,
"bundledDependencies": [
Copy link
Member

Choose a reason for hiding this comment

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

why is that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

ah ok, weird

Choose a reason for hiding this comment

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

node-pre-gyp needs to be a bundledDep (sadly) because otherwise it might not be usable (fully installed) by the time npm calls out to it. See mapbox/node-pre-gyp#162 for ideas about avoiding this (and being able to safely have node-pre-gyp dynamically installed and still available in time)

Copy link
Member Author

Choose a reason for hiding this comment

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

@juliangruber
Copy link
Member

+1

@ralphtheninja
Copy link
Member Author

iojs working again, merging.

@ralphtheninja ralphtheninja merged commit ddcc9ce into Level:master Jun 1, 2015
@ralphtheninja ralphtheninja deleted the prebuilt branch June 1, 2015 23:37
@ralphtheninja
Copy link
Member Author

Tests failed because I deleted my remote branch. Just fyi.

@ralphtheninja ralphtheninja removed the wip label Jun 1, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants