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

build: remove mdb from io.js #1023

Closed
wants to merge 1 commit into from
Closed

Conversation

jbergstroem
Copy link
Member

(this is part of adding a smartos builder in our jenkins cluster)

After upgrading to a newer v8 mdb is pretty much broken - even if using the latest updates from nodejs. If nodejs decides to update their v8 we can give it another go, but for now it's better to remove it than have it in our tree unsupported.

/R=@rvagg, @geek

After upgrading to a newer v8 mdb is pretty much broken - even if using the
latest updates from nodejs. If nodejs decides to update their v8 we can give it
another go, but for now it's better to remove it than have it in our
tree unsupported.
@rvagg
Copy link
Member

rvagg commented Mar 2, 2015

My suggestion to @jbergstroem was to gut this and if it needs to come back then Joyent is in a better position to do so and keep it updated.

https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/224/

/R+=@bnoordhuis @indutny

@rvagg
Copy link
Member

rvagg commented Mar 2, 2015

also LGTM as long as the other build slaves are OK with this too

@jbergstroem
Copy link
Member Author

The smartos build slave is currently not building correcty, see upstream bug here: https://code.google.com/p/v8/issues/detail?id=3935

@bnoordhuis
Copy link
Member

LGTM and I agree with the rationale.

@jbergstroem
Copy link
Member Author

To get mdb rolling again is a fair amount of work. Needs to be synced against v8 which is a moving target. I spent some time on this before talking to @rvagg and made progress, but I wouldn't be able to shoulder maintaining it. It'd be better to get proper upstream (as in joyent or anyone else intersecting Illumos/v8) backing before reintroducing it.

@jbergstroem
Copy link
Member Author

All good to merge?

bnoordhuis pushed a commit that referenced this pull request Mar 5, 2015
After upgrading to a newer v8 mdb is pretty much broken - even if using
the latest updates from nodejs. If nodejs decides to update their v8 we
can give it another go, but for now it's better to remove it than have
it in our tree unsupported.

PR-URL: #1023
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <rod@vagg.org>
@bnoordhuis
Copy link
Member

Thanks Johan, landed in 37bb1df with the commit log wrapped at 72 columns.

@bnoordhuis bnoordhuis closed this Mar 5, 2015
@jbergstroem
Copy link
Member Author

@bnoordhuis Thanks. Sorry about the commit text (esp the crypto test stuff). I'll fix my editor settings and improve from here on.

@bnoordhuis
Copy link
Member

@jbergstroem If it helps, I have autocmd BufEnter COMMIT_EDITMSG set textwidth=72 colorcolumn=+1 in my .vimrc (although the stock vim on my FC21 box is smart enough to understand what the commit log should like so it's not really necessary.)

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