From 88505bdbd28e91803710d052b7ae7819f6b05381 Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Thu, 3 Sep 2015 16:22:57 +1000 Subject: [PATCH] doc: add TSC meeting minutes 2015-09-02 PR-URL: https://github.com/nodejs/node/pull/2674 Reviewed-By: Colin Ihrig --- doc/tsc-meetings/2015-09-02.md | 418 +++++++++++++++++++++++++++++++++ 1 file changed, 418 insertions(+) create mode 100644 doc/tsc-meetings/2015-09-02.md diff --git a/doc/tsc-meetings/2015-09-02.md b/doc/tsc-meetings/2015-09-02.md new file mode 100644 index 00000000000000..36f3a9efb7ae9e --- /dev/null +++ b/doc/tsc-meetings/2015-09-02.md @@ -0,0 +1,418 @@ +# 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: _ + +## 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](https://github.com/nodejs/node/pull/2632) +* Inspecting Node.js with Chrome DevTools [#2546](https://github.com/nodejs/node/issues/2546) +* Node.js v4 Release Timeline [#2522](https://github.com/nodejs/node/issues/2522) +* doc: update COLLABORATOR_GUIDE.md [#2638](https://github.com/nodejs/node/pull/2638) + +### nodejs/build + +* Merge job overwrites metadata [#179](https://github.com/nodejs/build/issues/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](https://github.com/nodejs/build/issues/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](https://github.com/nodejs/node/pull/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](https://github.com/nodejs/node/pull/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](https://github.com/nodejs/node/issues/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](https://github.com/nodejs/node/issues/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