-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
[Converge] Re-enable mdb #2517
Comments
Assigning to @misterdjules, please close when this lands, we have until the end of this week to get in to 4.0.0. |
@rvagg I won't be able to work on this before the end of this week. |
any chance of getting it done in the next few days @misterdjules? |
@rvagg Probably not, since mdb support seems to have been removed completely in nodejs master. I'm starting to take a look at it now though, and I'll keep this issue updated. |
@misterdjules OK, we'll punt on v4.0.x but we have roughly one month before LTS gets locked so if we can get it in for a |
Rescheduling for 5.0.0 due date (when 4.0.0 will go into LTS) |
Quick update: mdb_v8 is being updated by TritonDataCenter/mdb_v8#24, and there's a pending upstream V8 change here: https://codereview.chromium.org/1296743003. @ofrobots Do you have some time to review https://codereview.chromium.org/1296743003? Otherwise, who should I ping to get some attention? |
@misterdjules The change looks good to me, but I'm not a V8 reviewer. You should add verwaest@chromium.org and possibly bmeurer@chromium.org as reviewers. If this needs to go into 4.0, I'd vote for floating the patch in Node.js with the expectation that upstream is likely to accept. This is not a risky change and should not affect anything other than post-mortem. /cc @bnoordhuis. |
nice work, if @misterdjules or someone else can float this in a PR against master in the next couple of days it can ship in 4.0.0 |
@ofrobots Thank you for the info, added verwaest@chromium.org and bmeurer@chromium.org as reviewers 👍 @rvagg To clarify things a bit: there's still some work to do to fix closures and scopes in mdb_v8 with the upgrade to V8 4.5.x. I should be able to make progress on that on Tuesday, as Monday is a holiday for me. It should not require a change to V8 itself though, but I'll keep you posted. |
@misterdjules Tag the pull request with the |
@bnoordhuis Already tagged, thank you for the info. |
closing this as I believe it's been achieved |
@rvagg Confirmed, thanks! |
Moving from nodejs/node-convergence-archive#28 (lots of background discussion there)
Summary from @davepacheco
mdb commits in 0.12:
878c40e1b3762a5f3ed48094f954fbee705f69ff mdb_v8: fix symbols not loaded properly
- nodejs/node-v0.x-archive@878c40eebfa7e350aed8f61b68a290be2e8fefca9db939e mdb_v8: add tests for ::findjsobjects -k KIND
- nodejs/node-v0.x-archive@ebfa7e34312f8d760bdf574b5a461e426331f0b4d298f18 mdb_v8: update for v0.12
- nodejs/node-v0.x-archive@4312f8dBackground: this was disabled in io.js because we didn't have anyone to maintain it. We need an expert to dig in and put the required metadata into this repo prior to v4 (i.e. soon!). Marking for the 4.0.0 milestone as an ideal but not necessarily a blocker if we can't get someone's time to help with this.
ping @nodejs/dtrace-mdb
The text was updated successfully, but these errors were encountered: