-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
doc: add CTC meeting minutes 2016-02-17
PR-URL: #5410 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
- Loading branch information
1 parent
25c01cd
commit 51bc062
Showing
1 changed file
with
240 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,240 @@ | ||
| # Node Foundation CTC Meeting 2016-02-17 | ||
|
|
||
| ## Links | ||
|
|
||
| * **Audio Recording**: https://soundcloud.com/node-foundation/ctc-meeting-2016-02-17 | ||
| * **GitHub Issue**: https://github.com/nodejs/node/issues/5274 | ||
| * **Minutes Google Doc**: <https://docs.google.com/document/d/1Y9p8EopccPOj1v4gDbnor2kvlHvkbHxankWuRKHssyo> | ||
| * _Previous Minutes Google Doc: <https://docs.google.com/document/d/1sAHu9jVh8Dn9gHFASOH56j2klzW-L6YP-DfKo1gO_6Y>_ | ||
|
|
||
| ## Present | ||
|
|
||
| * James Snell (CTC) | ||
| * Trevor Norris (CTC) | ||
| * Colin Ihrig (CTC) | ||
| * Brian White (CTC) | ||
| * Alexis Campailla (CTC) | ||
| * Bert Belder (CTC) | ||
| * Chris Dickinson (CTC) | ||
| * Shigeki Ohtsu (CTC) | ||
| * Steven Loomis (observer) | ||
| * Fedor Indutny (CTC) | ||
| * Rod Vagg (CTC) | ||
| * Ben Noordhuis (CTC) | ||
| * Nikita Skovoroda (observer) | ||
| * Ali Sheikh (observer) | ||
| * Evan Lucas (observer) | ||
| * Rich Trott (observer) | ||
| * Michael Dawson (observer) | ||
| * Seth Thompson (observer) | ||
| * Mikeal Rogers (observer) | ||
|
|
||
| ## Agenda | ||
|
|
||
| Extracted from **ctc-agenda** labelled issues and pull requests from the **nodejs org** prior to the meeting. | ||
|
|
||
| * Issue management: discuss caine again [#5246](https://github.com/nodejs/node/issues/5246) | ||
| * refactor src/node.js into internal files [#5103](https://github.com/nodejs/node/pull/5103) | ||
| * process: add 'warn' event [#4782](https://github.com/nodejs/node/pull/4782) | ||
| * CTC Membership Nominations [#4750](https://github.com/nodejs/node/issues/4750) | ||
| * buffer: add Buffer.from(), Buffer.alloc() and Buffer.allocUnsafe(), soft-deprecate Buffer(num) [#4682](https://github.com/nodejs/node/pull/4682) & Buffer(number) is unsafe [#4660](https://github.com/nodejs/node/issues/4660) | ||
| * Check in ICU into repo? [#3476](https://github.com/nodejs/node/issues/3476) - Steven R. Loomis - +1 on checking in <45M into repo? so Jenkins servers will never again have to download ICU | ||
| * Detect ICU data at startup? [#3460](https://github.com/nodejs/node/issues/3460) - Steven R. Loomis - +1 on ≈/node_modules/.node-icu ? | ||
|
|
||
|
|
||
| ## Standup | ||
|
|
||
| * James Snell: Working on a few PRs, the buffer change, the process.warning, looking at doc updates — backporting. Looking at other PRs, getting ready for conference. | ||
| * Trevor Norris: AsyncWrap bug on raspberry pi, getting two tcpwraps when creating a server instead of just one. | ||
| * Colin Ihrig: Review of issues tagged with child_process label and PR for adding options to process.send(). | ||
| * Brian White: fixing bugs, cleaning up code, more performance testing, reviewing PRs and issues. | ||
| * Bert: N/A | ||
| * Alexis: Mostly working on chakracore, moved the repo from ms/node, define participation guidelines; adding support in CI | ||
| * Chris Dickinson: Promises PR + WG (https://github.com/nodejs/promises). Microtask queue pluggability (https://github.com/chrisdickinson/node-1/pull/2). | ||
| * Shigeki Ohtsu: Reviewing a few PRs and issues and make my internal works. | ||
| * Steven R. Loomis: Intl packaging stuff | ||
| * Fedor Indutny: Reviewing PRs, fixing issues | ||
| * Rod: Promises, v5.x management, people & administrative stuff | ||
| * Ben: Promises PR, and ES2015 module interop, there’s a lot to take in, no fully formed opinions yet. | ||
| * Nikita Skovoroda: mostly comments and reviews, as usual. | ||
| * Ali Sheikh: Back from 2 weeks of vacation. Catching up on mail & jetlag. Progress on Sampling Heap Profiler in V8. Was involved in Error summit before vacation. | ||
| * Evan Lucas: Working on some http things (cleanup, etc.). Wrote CLI tool for submitting jenkins builds and viewing CI status. | ||
| * Rich Trott: flaky tests; updating ESLint; finding stale issues and closing, tagging, or taking other appropriate action | ||
| * Michael Dawson: Working through AIX issues, landing pr, working on setting up job/answering question on running V8 tests in Node tree, benchmarking infra/new benchmarks, discussion with Richard on post-mortem WG activity and work, trying to read issues with high traffic. | ||
| * Mikeal Rogers: Foundation stuff, DCO 1.1 patch. | ||
| * Seth Thompson: Traveling the past week or so; working on revamping profiling and metrics on our perfbots, and trying to get Node building on the chromium bots, to assist with the benchmarking WG. | ||
|
|
||
| ## Review of last meeting | ||
|
|
||
|
|
||
| * Revert "fs: deprecate fs.read's string interface" [#5163](https://github.com/nodejs/node/pull/5163) & fs: add a temporary fix for re-evaluation support [#5102](https://github.com/nodejs/node/pull/5102) | ||
| * buffer: add Buffer.from(), Buffer.alloc() and Buffer.allocUnsafe(), soft-deprecate Buffer(num) [#4682](https://github.com/nodejs/node/pull/4682) & Buffer(number) is unsafe [#4660](https://github.com/nodejs/node/issues/4660) | ||
|
|
||
| ## Minutes | ||
|
|
||
| ### Issue management: discuss caine again [#5246](https://github.com/nodejs/node/issues/5246) | ||
|
|
||
| Fedor: A year ago I submitted an idea, and we tried it for a bit, to use a GH bot to help us manage issues on GH. This was to address the pace of issues. The idea was to introduce a bot that automatically assigns issues to people based on tags & user responses to questions. It didn’t take off at the time, the reason being we didn’t want to be too automatic. Chris had some issues with it. I wanted to see if we could take another look. | ||
|
|
||
| Mikeal: Today GH slipped in an issue template feature. We may want to investigate that. | ||
|
|
||
| Fedor: Indeed this should solve the thing for us. We can ask folks to mention some specific working groups. | ||
|
|
||
| Bert: I also had objections to the first version of caine, I felt like it would only add noise — it would still ask questions even if the issues had been answered. I would suggest we start with the template then look at what caine could add to it. Maybe autotagging? | ||
|
|
||
| Mikeal: We have a lot of folks tagging, but we don’t have a good way for limiting email notifications. Maybe autoassign tags to users? | ||
|
|
||
| Bert: I am not opposed to this. | ||
|
|
||
| Mikeal: Maybe it should never comment. | ||
|
|
||
| Fedor: I kind of disagree with this. My workflow: I was actively using GH teams to filter emails. If the bot would be able to assign the tags — and cc teams in a single comment. | ||
|
|
||
| Chris: I’d second that. | ||
|
|
||
| Rod: I’d advise starting light; it may get pushback if it comments too much. | ||
|
|
||
| Ben: What exactly would you change? | ||
|
|
||
| Fedor: Autotagging based on responses in template. We’ll ask folks to name the part of core in the template, and add a mention of the relevant GH team so that they get notifications. | ||
|
|
||
| Mikeal: If someone wanted to take on an interesting side project, feeding issues + tags into ML would be pretty cool. | ||
|
|
||
| Bert: I think that’s a really interesting idea actually. | ||
|
|
||
| Mikeal: Maybe some IBM folks that have access to Watson could do this? | ||
|
|
||
| Bert: Maybe we could give interested folks an account? | ||
|
|
||
| James: [CD: Paraphrasing: It seems possible.] If anyone’s interested in this, send me a note offline. | ||
|
|
||
| ### refactor src/node.js into internal files [#5103](https://github.com/nodejs/node/pull/5103) | ||
|
|
||
| Jeremiah tagged this one. Tagged because it’s heavy-handed into splitting `src/node.js` into sub-files in the internal directory. | ||
|
|
||
| James: It is a big change, but it seems to be one that’s fairly safe. I didn’t notice any things that would obviously break. Given the code that it does touch, it’s worth being conservative and careful pulling it in. I’m +1 on it, but I think we need to be careful. | ||
|
|
||
| Chris: Naming issues. | ||
|
|
||
| Ben: Memory overhead. Script objects have a moderately sized footprint, so that’s a thing to keep in mind. | ||
|
|
||
| Rod: I think we could name the files with `bootstrap-`. With regards to any objections to doing this… There’s the usual fear re: size of changes, memory, breaking people’s workflows. Is there anything else to discuss here? | ||
|
|
||
| Michael: Suggest running the footprint benchmark to get a feel. | ||
|
|
||
| Ben: Will post on GH. | ||
|
|
||
| [CD: Moved to GH] | ||
|
|
||
| ### process: add 'warn' event [#4782](https://github.com/nodejs/node/pull/4782) | ||
|
|
||
| James: Came from how we do warnings currently, which is to drop it out into console.error now. This restructures it a bit to put it into a warning event on the process object, allows users to listen to those warnings. Mainly looking for review. Existing command line arguments are still supported, but messages are changed a bit. Adds ability for us to add warning for security purposes or bad practices. Process.emit ‘warning’ allows users to emit their own warnings. It came up via the buffer changes — where we could issue a warning for users using the new Buffer constructor in an unsafe way. Just looking for review, nothing too controversial. | ||
|
|
||
| Trevor: The buffer constructor needs to have to have the same signature as Uint8Array for subclassing purposes. | ||
|
|
||
| Michael: This captures all output to stderr from core? | ||
|
|
||
| James: No, just the warnings, like the EE maxlisteners warning. The default behavior is to print it to console, but it may be overridden by user handlers. Folks from electron have said that this is something that they like. | ||
|
|
||
| Rod: Any concerns or objections raised in GH? | ||
|
|
||
| James: Not thus far — tty check raised by Jeremiah | ||
|
|
||
| Trevor: Does it sent the output to stderr/stdout only or could one provide a file descriptor? Looks like you're building a messaging system... | ||
|
|
||
| James: Default behavior is to print to stderr, but you can turn off this behavior by installing your own listener. | ||
|
|
||
| Chris: Are we sure no one’s using process.emit(‘warning’)? | ||
|
|
||
| James: Did a search and came up with no results. | ||
|
|
||
| Chris: Excellent. | ||
|
|
||
| Rod: It’s more than adding an event, it’s worth taking a look. | ||
|
|
||
| James: [CD: my internet is flaking out] Mostly trying to get it on folks’ radar. If folks can take a look and provide their feedback. | ||
|
|
||
| Rod: FWIW I think this is a great change. Unifies and adds some nice functionality around these warnings. | ||
|
|
||
| ### CTC Membership Nominations [#4750](https://github.com/nodejs/node/issues/4750) | ||
|
|
||
| Rod: Any objections to putting this to a vote next week? | ||
|
|
||
| [crickets] | ||
|
|
||
| ### buffer: add Buffer.from(), Buffer.alloc() and Buffer.allocUnsafe(), soft-deprecate Buffer(num) [#4682](https://github.com/nodejs/node/pull/4682) & Buffer(number) is unsafe [#4660](https://github.com/nodejs/node/issues/4660) | ||
|
|
||
| James: added ability to specify byte offset and length, both to `.from` and [CD: missed this] | ||
|
|
||
| James: No significant objections, except to allocUnsafe which folks really don’t like the naming of. This is shaping up, we should be able to get that landed fairly soon, just need to hear from this group if there’s objections on naming or status of this? | ||
|
|
||
| Rod: Myself and Trevor still don’t like the name. I’m not willing to object too strongly to that. | ||
|
|
||
| Bert: The names are ugly, but I don’t want to bikeshed. Same as Rod, I guess. | ||
|
|
||
| James: If I could ask folks to do a final review, it’s a big PR. All buffer uses in core are updated to use this. Let me know if there’s any objections. I’d like to get this landed before too long. | ||
|
|
||
| Trevor: The documentation changes are what took me the longest to get through. | ||
|
|
||
| Rod: Documenting this stuff has caused problems in the past. | ||
|
|
||
| ### Check in ICU into repo? [#3476](https://github.com/nodejs/node/issues/3476 ) - Steven R. Loomis - +1 on checking in <45M into repo? so Jenkins servers will never again have to download ICU | ||
|
|
||
| Steven: Speaking of massive PRs! I haven’t created the PR for this. I have been doing some slicing on ICU, I have now a 45mb branch that includes all of ICU, enough to build a full set or a full-icu, or a small icu, and removes the requirements for the servers to download anything at all at build time. I’m asking for sign-off on this. It would make small-icu the default for `./configure`. | ||
|
|
||
| Steven: it will be located in source: `deps/icu` | ||
|
|
||
| Trevor: How often would this need to be updated? | ||
|
|
||
| Steven: [CD - missed this] | ||
|
|
||
| Trevor: A good chunk of the deps/v8 updates include tarballs. | ||
|
|
||
| Ben: Does it write the data files at compile-time? | ||
|
|
||
| Steven: This just checks in the binary blobs. | ||
|
|
||
| Ben: In that case Trevor makes a good point. | ||
|
|
||
| Steven: The data file is not compressed, it does compress fairly easily. (59% by gzip) | ||
|
|
||
| Trevor: One of the original reason for not landing this was that the source files were 200mb, but when they’re compressed down they’re 45mb. Git works better with the former. We’d almost have to do a test to see which is a more sustainable in the long run. | ||
|
|
||
| Steven: Including the source data means that the source data has to be compiled which takes a long time. (+75M source + tools) | ||
|
|
||
| update schedule - http://source.icu-project.org/repos/icu/data/trunk/meta/xml/icumeta.json | ||
|
|
||
| Trevor: We’ve discussed all of this. If we just include English, how do we present this on the website? There’s a lot of contention | ||
|
|
||
| Rod: I’ve had trouble getting my license updater to work because not having it in-repo breaks things. On the other hand, the repo is growing at a fast pace, and it’s pretty bad UX (have you ever tried cloning the Mozilla repo?) | ||
|
|
||
| Chris: It seems like including the full source | ||
|
|
||
| Steven: We could do the 20mb, only 3mb of which is the binary blob. And we could pick it up every year. full data available via npm (see next issue) | ||
|
|
||
| Ben: what about security releases? Do those require updating the binary blob? | ||
|
|
||
| Steven: No. | ||
|
|
||
| Trevor: My only concern is the binary blobs. | ||
|
|
||
| Rod: Let’s start with the small binary blob will allay a lot of our concerns here. | ||
|
|
||
| Steven: I may be able to slice the 20mb more. This is with no configure source changes to ICU. It may be further reducible. | ||
|
|
||
| ### Detect ICU data at startup? [#3460](https://github.com/nodejs/node/issues/3460 ) - Steven R. Loomis - +1 on ≈/node_modules/.node-icu ? | ||
|
|
||
| Steven: This is the companion to the above. The basic idea here is that I went through the resolver code and what I’m proposing is having the data files in a directory inside of `node_modules/.node-icu`, installed by npm module or by some other installer could install it to that directory. | ||
|
|
||
| Trevor: Has there been any thought to some sort of integration with npm? | ||
|
|
||
| Steven: I hadn’t thought about that. Nathan had commented much earlier, concerned with linkage between Node and npm. This only uses the node_modules directory, not npm itself. | ||
| Chris to point CLI team at issue. | ||
|
|
||
| ### Other business | ||
|
|
||
| _Discussed decision making process around the Promises PR_ | ||
|
|
||
| ## Next Meeting | ||
|
|
||
| 2016-02-24 |