PR-URL: https://github.com/nodejs/node/pull/2674 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
17 KiB
Node Foundation TSC Meeting 2015-09-02
Links
- Audio Recording: https://soundcloud.com/node-foundation/tsc-meeting-2015-09-02
- GitHub Issue: https://github.com/nodejs/node/issues/2654
- Minutes Google Doc: https://docs.google.com/document/d/1rXBdtsD9PJTExNXgzNZ9bez9oOjW45kLPjun4zdt0dY
- Previous Minutes Google Doc: https://docs.google.com/document/d/1aB76ClCgdjZUw3p-gHq9j6YwU11zWgJ4pmYPEWk6gjE
Agenda
Extracted from tsc-agenda labelled issues and pull requests in the nodejs org prior to meeting.
nodejs/node
- deps: update v8 to 4.5.103.30 #2632
- Inspecting Node.js with Chrome DevTools #2546
- Node.js v4 Release Timeline #2522
- doc: update COLLABORATOR_GUIDE.md #2638
nodejs/build
- Merge job overwrites metadata #179
Minutes
Present
- Rod Vagg (TSC)
- Brian White (TSC)
- Steven R Loomis (TSC)
- Fedor Indutny (TSC)
- Bert Belder (TSC)
- Colin Ihrig (TSC)
- Trevor Norris (TSC)
- James M Snell (TSC)
- Chris Dickinson (TSC)
- Jeremiah Senkpiel (TSC)
Standup
- Rod Vagg (TSC): node v4 prep, new website
- Brian White (TSC): issue/PR reviewing, working on JS DNS implementation some more
- Steven R Loomis (TSC): syncing with james, creating text for renaming of language groups (#2525), intl triaging, commenting on joyent/node issues
- Bert Belder (TSC): indisposed until end of october
- Colin Ihrig (TSC): reviewing issues/prs
- Trevor Norris (TSC): prs/issues, finagling the new build pr landing tool
- James M Snell (TSC): old prs in joyent/node cleaned up (64 remaining, wow), citgm tool updates, child process arguments pr, nodeconf.eu
- Michael Dawson (TSC): AIX support PR, some triage with Devin, preparing for NodeConfEU
- Chris Dickinson (TSC): Static analysis work (ongoing), docs (slowly), npm
- Jeremiah Senkpiel (TSC): v4 release prep
- Fedor Indutny (TSC): v8 arraybuffer perf, patch landed on v8; reviewing PRs
Jenkins merge jobs always overwrites PR-URL and Reviewed-By #179
Trevor: currently if you submit a job to land a pr, it will always remove all existing metadata and re-apply it based on the info passed to the jenkins build. from convo, good fix is to let old metadata persist & only append new metadata if it’s entered, if no metadata entered, nothing appended Issue with current state: current system does not work for cherry-picks onto release branch — will wipe metadata and replace it Goal: solidify with TSC that this is a good path forward
James: absolutely, +1
Jeremiah: clarification on which branches this applies to
Rod: cherry-picks are becoming increasingly difficult on iojs anyway, so the pr landing job may be master-specific
James: no mixing of “with pr tool” and “manually”, things go bad quickly that way
Trevor: cherry-pick PRs are accumulating too many cherry-picks; runnin it through the system manually doesn’t add useful info
Domenic: description of chromium
- tests are run after landing
- no one is allowed to land commits if the build is broken on a branch
- sheriff of the day is responsible for fixing the issue
- in practice it’s painful, but it’s important for keeping things sane
Trevor: this would make us better about flaky tests, also, admiration for how sporadically our flaky tests break
Jeremiah: trying to do this now before v4 seems like a bad idea
Rod: we’re smoothing out a little bit; circle back to original topic, we’re kind of talking about the larger issue, let the trial of the tool continue and come back to the discussion later. specifically: ask that metadata not be overridden in a github issue
Domenic: addressing jeremiah’s concern: should we not run the trial while releasing v4?
Trevor: agreement — have felt pain from this as we march to v4
Jeremiah: stuff that gets released to people is in the release branches, if we’re not using the pr tool on that branch, what is the point of using this tool?
Rod: this will become more of a problem as we have more extended and LTS
Domenic: clarification: intent of ci is to make sure tests pass everywhere
Trevor: this tool only runs a subset
Rod: since we branched 3.x there’s 4 commits in the tree that have faulty/missing metadata — like that the pr tool is enforcing this metadata, maybe move to using github status api for telling us whether the metadata is OK
Trevor: clarify: contributors have landed commits that lack metadata?
Rod: yep
Trevor: argh
Jeremiah: github suggested we use the status api
Trevor: status api autoruns?
Rod: you hook it up to webhooks, has in-progress and pass/fail
Trevor: TJ can verify that devs destroy jenkins (multiple force pushes explode the jenkins box with so many jobs)
Rod: we can cancel on force push (or add a “trevor is pushing, wait half an hour to run the tests” exception, har har)
Jeremiah: you can tell if it’s been run on the last commits
Rod: multiple statuses — up to 100? — “this pr is passing on these platforms! this pr has the right metadata. this pr lints well.”
Rod: let’s move on!
James: what’s the action item?
Rod: don’t feel that we should cancel the trial
Trevor: two prs that prevent segfaults because of other flaky tests that have to be reapplied
James: in the 4.x branch do we need to use test-pr job to land commits?
Rod: nope, cherry-pick onto those branches, only use the tool on master
Jeremiah: it’s weird to use it on the unstable branch. I guess we could make it better for release branches in the meantime
- ci-run middle of hte night us time this has a
Rod: vote for who whether we’d like to put this on hold
Jeremiah: +0 put on hold
Rod: who would like to continue it?
...crickets...
Trevor: give me another day
James: one PR took 8 hours to land last friday
Jeremiah: they take about an hour to land right now
Rod: they are getting quicker, adding raspberrypis to the cluster. good incentive to make node startup time quicker! arm startup times have gone down over the last month or so
Rod: let’s move on
doc: update COLLABORATOR_GUIDE #2638
Jeremiah: Alexis wanted to put the pr tool into the collaborator guide should punt on this right now notifying collaborators on issues is more effective messaging than the docs
Rod: let’s hold off
deps: update v8 to 4.5.103.30 #2632
Rod: looking at the possibility of jumping to 4.5 for the v4 release it’s ready to merge. ofrobots has been doing the work, it’s good to go, there are pros and cons whether we stick with 4.4 or 4.5 for this release & associated LTS has anyone done any work on testing NAN?
Bert: what breaks? what are the cons?
Domenic: looking at v8 by code inspection, there are no breakages, just new deprecations — should in theory work — I don’t think anyone’s been testing it though
Bert: can this v8 release be supported for LTS? we don’t know if this is better or worse than 4.4? maybe ask goog eng
Trevor: they will say “Stay current”
Bert: I think I agree with them — with no other info, picking the latest version seems best
Michael: Z targets 4.4 — just recently got it fully clean — a move to 4.5 would cause a delay on getting release to community
James: that’s just Z, not PPC, yes?
Rod: how much work?
Michael: going 4.3->4.4 took about a month
Jeremiah: 4.3->4.4 breakage delta was much bigger
Rod: that’s the external API though
Michael: 7-8 patches/week? if we have to upgrade it will be extra work + delay for us
James: if we’re sitll looking at LTS in early-mid oct, it’s going to give us a month to prove out v8 4.5
Domenic: we’ve been proving out 4.5 for 12 weeks
Rod: we’re talking node here, though we’d have google support for an extra month. my concern is: we need v4 out now, latest by monday, this is a quick change and there’s potential for breakage
James: we decided before that we weren’t going to make such a big change so quickly before a release. i agree with rod, but such a big change makes me nervous — making sure that nan et al works with 4.5 is a concern
Jeremiah: we’re not going to be the ones to find the flaws in 4.5; want clarification on nan major/minor/patch?
Rod: minor
Domenic: should require no changes
Rod: benjamin (kkoopa) is on the positive side of this, he wants to see us move to 4.5, and is willing to put in the work to do this
Trevor: not that it matters much, but node has taken a hit from v8 in terms of perf recently, 4.5 has some improvements, fedor wrote an easily backportable patch as well, there are perf advantages
James: it makes me nervous that we’ll have enough time to test things on the node side before LTS — if we’re comfortable saying we have a plan that we think exercises this before going LTS, I’m more ok
Jeremiah: technically we have over a month
Rod: vote
who is leaning sticking to 4.4
James + michael + steven
4.5:
fedor, colin, jeremiah, brian, chris
Rod: clarifying concerns
Michael: share concerns with james, but also Z
Rod: longer we have to support our own V8, the harder it is, and another 6 weeks of support would be great great
James: I’m -0
Rod: spending some time today testing some existing addons taht have upgraded to nan@2. if there’s breakage, that tells me nan’s not ready, and that should be taken into account
Domenic: if nan’s not ready, we don’t upgrade
Rod: heading for 4.5, but with final go/no-go coming from nan testing; where are you at on this bert?
Bert: I am happy to upgrade if you land it and nothing changes — which sounds like it might be the case (sans deprecation warnings)
Rod: want ben’s opinion, but there may be no chance of that?
Bert: it seems silly to go with a version that is almost already out of support, but OTOH, on the scale of LTS lifetime, 6 weeks is not really significant
I wouldn’t worry too much about actual bugs in v8, about as likely in 4.5 as in 4.4; LTS is not about “no bugs”, it’s about “We support it”
Rod: we want a solid release, though
Bert: the LTS will also have a beta for a while, yes?
Rod: the beta period for LTS is the stable line
James: first LTS will have a beta of a little over a month, in the future 6 months
Bert: realistically a month is a longer beta than we’ve ever had before
Rod: 0.12 had a beta of 2 years
{zing} :trollface:
Bert: a beta with a lot of api changes, which is explicitly what we’re not going to do
Bert: we need to move on
Jeremiah: 4.5 brings arrow functions and that will make many people very happy
Rod: it’s a good sales pitch!
Rod: I’ll make the call.
Node.js v4 Release Timeline #2522
Rod: status update: v4 was to get out by Thurs, slipping on that, monday is the release date, cannot afford to slip any further
Jeremiah: what’s actually slipping other than mdb?
Rod: we’re not going to get mdb into v4, encourage before LTS, but that’s the best we can do, child process argument type checking where are we at on that?
Trevor: it’s not ready
James: working on that PR right now
Rod: great, that feels like a fairly light one, process.send is the other one; ben’s on holiday, where are we at with that pr of his
Jeremiah: multiple people have soft signed off on it
Colin: weren’t there some strange ci failures with it?
Rod: do not know, does someone want to put their hand up for that?
Trevor: what’s the PR number?
Jeremiah: #2620
Jeremiah: Trevor +1’d it
Jeremiah: OH! that’s the one with the weird test failures we could not reproduce the breakage
Rod: this seems like it can be run again and it won’t show flaky tests: maybe we should just re-run it
Trevor: that really sucks when you’re trying to land 3 tests
Rod: test runner should auto-rerun to make sure
Trevor: oh, gotcha
Rod: that’d be really quick for flaky tests
Trevor: I’ve already reviewed and LGTM’d it, I’ll look at running it through the land-ci job
James: looked at it earlier today, nothing concerning stood out
Rod: no RC’s are out! that’s a problem! we’ve renamed the executable. I wanted to have a period of time where folks could download (with a working node-gyp) — we need this period of time. I’ve been working on that, some bits and pieces to get things to the right place on the server. first build had a broken OS X installer. there are a few yaks yet to shave, so that’s why we can’t get this out by thursday
Rod: steven: could you confirm that 3.x has INTL enabled?
Steven: will look into, take as action item (confirmed!)
Rod: not going to tick that off till we confirm, but I think that one’s done
Rod: how’s everyone with the timeline? rcs ASAP + release on monday
James: sounds reasonable, yes. will take as much time as possible on monday. several of us will be at nodeconf, so that complicates
Rod: I’m using monday in airquotes, it’ll be tuesday for me. other thing: smoke testing, citgm, I’ve been using it, and there are a lot of weird failures, after v4 we’re going to have to switch modes to make the ecosystem happy with it
Rod: let’s move ahead with that plan; nodeconf.eu is going to take up a lot of time for a lot of folk. congrats to everyone that contributed to new website, it seems really smooth
Inspecting Node.js with Chrome DevTools #2546
Rod: opened by member of v8 team, extracting debugger code from blink and making it so all embedded v8 projects can use it
Trevor: gist is: devtools team is considering pulling inspector out of blink into its own project so we could consume it; domenic can correct me wherever I’m wrong here,
Domenic: “making node work the same as android” able to point chrome dev tools at node processes to debug ‘em
???: only works with chrome
Trevor: OTW API we consume have remained fairly stable, even if the API does change, chrome dev tools is capable of pulling in a previous version, making sure it’s possible to use with LTS releases
OTW wire is generic, consumable by anyone, chrome dev tools is a key consumer, but anyone can consume (new consoles can compete!)
cons:
APIs going to need to be tightly integrated — needs to know about all async events add conditionals to e.g. nextTick + timers
Domenic: add something to MakeCallback?
Trevor: we have to add it to that + on the way back in
Domenic: this is for async stack traces
Trevor: we don’t have to have it
Bert: that’s awesome, because we have to grow a single entry point (opposite MakeCallback)
Trevor: not going touch nextTick/timers, though — need to wrap each (because those APIs don’t roundtrip to C++ for each callback)
Bert: we don’t have to do that immediately though, but eventually it’d be a good idea
Domenic: could also move to microtask for nexttick — integrating with domains sounds hard, then we get those events for us
Trevor: would you be able to do unhandledRejections?
Domenic: we already have this for promises
Rod: if we could use this as a good justification for removing domains, that’d be awesome
Trevor: next point is pro and con: importantly, we’d have to support websockets natively in node
Jeremiah: we could just keep ‘em private
Trevor: that would just anger people
Domenic: you could bundle them and hide them while you’re not clear on the api, iterate and expose later
Rod: that’s a good point
Trevor: the interesting flip to this: they need to be running off the main thread, in a debugger thread — it can all be written in C++
mscdex: any way to avoid websockets?
Trevor: no
Bert: is the protocol really that complicated? I don’t think so — especially sans full HTTP and WSS — just websockets though is probably not difficult
Trevor: necessary because it’s the protocol that chrome dev tools uses
Rod: decision points: are we being asked to get on board?
Trevor: them removing the inspector from blink depends on whether we’ll use it if they do it.
Rod: debugging in node sucks, we are shipping with substandard debugging in v4, we don’t have the contributors + collaborators to make this better, so if we can lean on v8 to do this, I’m +1
debugging is important enough to override “small core”
Bert: we’re not adding the full inspector, just the protocol
Trevor: wasn’t completely clear on how much API we’d need to implement
Domenic: we should get clarification on this
Trevor: if we can implement this in segments — if we’re all +1 on this, we need to create a new isolate, but …
Domenic: would the workers pr solve this?
Trevor: probably overkill. I’m +1 on the debugger
Bert: I propose we don’t vote
{broad assent}
Rod: trevor + domenic: you’ve got enough input
Domenic: only worry is finding someone to devote some of their daytime hours to implement websockets
Rod: nodesource could contribute, related to our interests
Trevor: even if we don’t say yes today, taking inspector out of blink is a complicated process and will take time
Rod: anything else?
Next Meeting
September 9th