PR-URL: https://github.com/nodejs/node/pull/8423 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
12 KiB
Node Foundation CTC Meeting 2016-08-24
Links
- Audio Recording: TBP
- GitHub Issue: #8242
- Minutes Google Doc: https://docs.google.com/document/d/1mkOUQ-M3s6zV2ige00hj6IIlT5oGQcw1USeliRqRDSk
- Previous Minutes Google Doc: https://docs.google.com/document/d/10fKK2lyZrxJ1Gt4zJvVSIsfmTGVD547vJJOp6eVKk-k
Present
- Anna Henningsen @addaleax (CTC)
- Bradley Meck @bmeck (observer/GoDaddy/TC39)
- Сковорода Никита Андреевич @ChALkeR (CTC)
- Jeremiah Senkpiel @Fishrock123 (CTC)
- Fedor Indutny @indutny (CTC)
- James M Snell @jasnell (CTC)
- Josh Gavant @joshgav (observer/Microsoft)
- Michael Dawson @mhdawson (CTC)
- Julien Gilli @misterdjules (CTC)
- Brian White @mscdex (CTC)
- Jenn Turner @renrutnnej (observer/Node.js Foundation)
- Rod Vagg @rvagg (CTC)
- Steven R Loomis @srl295 (observer/IBM/ICU)
- Myles Borins @TheAlphaNerd (observer)
- Trevor Norris @trevnorris (CTC)
- Rich Trott @Trott (CTC)
Standup
- Anna Henningsen @addaleax (CTC)
- Issues & PR review
- Couple of own PRs
- Bradley Meck @bmeck (observer/GoDaddy/TC39)
- Started rewrite/overhaul of Module EP
- Сковорода Никита Андреевич @ChALkeR (CTC)
- Comments on issues and PRs, nothing significant
- Started documenting my npm tooling
- Jeremiah Senkpiel @Fishrock123 (CTC)
- Minor PR & Issue things
- Review of the Promises Warnings thing
- Fedor Indutny @indutny (CTC)
- Reviewing PRs, nothing fancy
- James M Snell @jasnell (CTC)
- PR review
- Josh Gavant @joshgav (observer/Microsoft)
- Issue review, keep CTC notes in order
- Michael Dawson @mhdawson (CTC)
- Away on vacation last week
- Fixed up problems with acme air benchmarks
- Investigated AIX machine issues (firewall problem)
- misc PR review/land
- catching up on issues
- starting to add new PPC machines (hopefully final set)
- Julien Gilli @misterdjules (CTC)
- making libuv test suite pass on SmartOS
- Brian White @mscdex (CTC)
- My continuing mission: to explore strange new deopts, to seek out new JS and new optimizations.
- Reviewing PRs, commenting on issues
- Jenn Turner @renrutnnej (observer/Node.js Foundation)
- Observing in prep for a newsletter on Node community with Node.js Foundation
- Rod Vagg @rvagg (CTC)
- Taking it easy, some discussion, some meetings
- Steven R Loomis @srl295 (observer/IBM/ICU)
- Achievement unlocked: getting to CTC call
- Myles Borins @TheAlphaNerd (observer)
- (on vacation)
- Trevor Norris @trevnorris (CTC)
- async wrap; considering changing the name of public API require.
- Rich Trott @Trott (CTC)
- Multiple issues causing CI yellow and red this week. Will file an issue for next meeting for things we may wish to do differently.
- Helped three first-time contributors get their first commits in. (Well, two of them so far.) Some documentation changes have/will come from this.
- Lots of small PRs to refactor tests.
Agenda
Extracted from ctc-agenda labelled issues and pull requests from the nodejs org prior to the meeting.
nodejs/node
- CTC membership nomination: @TheAlphaNerd #8058
- buffer: hard-deprecate Buffer constructor #7152
- fs: don't alter user provided
options
object #7831 - fs: undeprecate existsSync; use access instead of stat for performance #7455
- doc: add Google Analytics tracking. #6601
- Introduce staging branch for stable release streams #6306
- Seek legal advice on LICENSE and copyright blocks in code #3979
nodejs/api
- Landing node-eps for ABI stable module and location for PoC code #28
Previous Meeting
Extracted from ctc-agenda labelled issues and pull requests from the nodejs org prior to the meeting.
- CTC membership nomination: @TheAlphaNerd #8058
- vote next week
- buffer: hard-deprecate Buffer constructor #7152
- separate into 2 PRs (one for deprecation without
new
, one for deprecation even withnew
) - revisit next week
- separate into 2 PRs (one for deprecation without
- Revert "fs: add a temporary fix for re-evaluation support" #6413
- Discussion continuing in GitHub.
- Introduce staging branch for stable release streams #6306
- Seek legal advice on LICENSE and copyright blocks in code #3979
Minutes
CTC membership nomination: @TheAlphaNerd #8058
@trott: Any objections?
Vote: Unanimous yes. No “nos” or official “abstains.”
Next steps:
- @rvagg to onboard @thealphanerd.
buffer: hard-deprecate Buffer constructor #7152
@addaleax: See https://github.com/nodejs/node/pull/7152#issuecomment-241355246
@jasnell and @rvagg both posted following @addaleax, she agrees.
@addaleax: Not in favor of dropping constructors which don’t match UInt8Array constructors.
@jasnell: What is the use case that requires Buffer
be subsclassable?
@trevnorris: It cleans up the code if we can extend directly from UInt8Array. Also, community members have requested the ability to extend Buffer.
@addaleax: We could work around that, we could export something from require(‘Buffer’)
which is subclassable.
@trevnorris: But your constructor name won’t be right.
@addaleax: I posted a proof of concept, I think everything is going to be right, needs perf evaluation.
@trott: One issue is whether and when to deprecate Buffer
without new
keyword. Any dissent on that?
@jasnell: Haven’t seen any dissent.
@trott: Can we finalize on that?
@trevnorris: to extend from UInt8Array all we need to do is deprecate Buffer
without new
.
@jasnell: We all agree about removing new
- hard deprecation.
@joshgav: Can you clarify what is meant by “soft” and “hard” deprecation?
@jasnell: "soft" means documentation only. "hard" means throw an error by default, or with command-line flag print a warning instead.
@trott: Other question re deprecating constructors with new
is not as clear. Do we need to figure this out now?
@jasnell: Recommend waiting, it’s more nuanced.
@addaleax: I agree. Maybe in v8.x.
@trevnorris: We can’t flatly deprecate all use cases of new Buffer(...)
.
Next steps:
- Land hard deprecation of
Buffer
withoutnew
now. - Other conversation continue in GH, remove ctc-agenda label.
fs: don't alter user provided options
object #7831
@FishRock123: while using util._extend
would be a semver-major break, does it actually break anything?
@indutny: It’s just a side effect, not major.
@FishRock123: should still be a semver-major change, but not something to worry about.
@rvagg: We’ve landed these types of changes in the past without waiting for semver-major.
@indutny takes back his words, it’s semver-major.
@trott: Seems pretty low risk for a semver-major.
@indutny: yes
@trott: post comments in GH if you have concerns, otherwise semver-major change is okay. What else?
@rvagg: This is just due diligence, there’s already use of utils._extend(...) in there.
@chalker: why did we have a test which passed an Object.create()
-ed options object to it? [No objections].
Next steps:
- CTC approved semver-major change. Remove ctc-agenda, continue with usual process.
fs: undeprecate existsSync; use access instead of stat for performance #7455
Action requested: Is it okay to undeprecate existsSync()?
@Fishrock123: There is a use case for a file which exists but has no content - .git/rebase-apply/rebasing
. Existence itself is a flag.
Also includes change to use fs.access
, I don’t think that’s reasonable.
@rvagg: only undeprecates existsSync
, not exists
.
@chalker: I’d be okay with that.
@trott: Postpone till next week to allow for review?
Next steps:
- First item on next week’s agenda.
doc: add Google Analytics tracking. #6601
@FishRock123: Was this fixed to not include GA for local usage?
@chalker: Ben isn’t here and has opinions.
@rvagg: Marketing and web site team want analytics to understand how docs are used. The objections are about user privacy, and there was a suggestion that we manage our own analytics. But there was a non-trivial cost.
@rvagg: Only for the online docs, not the tarball etc. I’m okay with this.
@trott: It’s a legitimate need for access, I have no concerns.
@trevnorris: Is this to put analytics tracking inside the docs repo?
@Fishrock123: PR replaces a line and puts in the GA link/script.
@rvagg: Has to be included via a special flag to make. Will only be set when building for web site.
@misterdjules: A DNT flag has been added too, that may address some concerns.
@Chalker: DNT flag makes this acceptable. What will we use the stats for? To improve docs?
See https://github.com/nodejs/node/pull/6601#issuecomment-224002869
@rvagg: My only concern is moving forward without @bnoordhuis, who was the main holdout based on strong privacy objections.
@indutny: I don’t like the idea of being tracked. Considering that other parts of web site do tracking it may be more acceptable. An important question is what people will have access to these statistics? Has it been made clear?
@rvagg: Good question. Probably at least @mikeal.
@trott: Raise these questions in GitHub. We’ll put this back on the agenda for a quick resolution next week.
Next steps:
- Raise above questions and discuss in GH.
- Review for quick resolution next week.
Introduce staging branch for stable release streams #6306
Defer to next week when @thealphanerd is back.
Next steps:
- Defer to next week.
Seek legal advice on LICENSE and copyright blocks in code #3979
@rvagg has action to follow up, particularly on LICENSE file.
Next step:
- @rvagg to remove
ctc-agenda
label
Landing node-eps for ABI stable module and location for PoC code #28
@mhdawson: EP has a view of what we’re trying to do, it’s definitely not the final version. But having it only as a PR makes it harder for others to collaborate on it. Can we merge it in draft status? Or perhaps move to another forum where we can collaborate more easily?
@misterdjules: Node-EPs repo says it’s okay to commit a draft.
@mhdawson: Then I’ll do that.
@mhdawson: Progress on POC code in a personal GitHub org, would like to put somewhere more visible, such as a repo in github.com/nodejs, or a branch in github.com/nodejs/node.
@Fishrock123: Maybe we should consolidate all experimental stuff (e.g. this, HTTP/2) in a nodejs/experimental repo?
@misterdjules: Are there concerns with new branches on main repo?
@Fishrock123: If there will be a lot of PRs and issues related to this branch it may block up the issue tracker.
@mhdawson: Issues will continue in nodejs/api repo.
@indutny: I don’t want to establish a precedent of having experimental branches in main repo. We had problems in past with cleaning these up. Having in a separate repo should simplify things.
@mhdawson: Would nodejs/node-experimental make sense, with different branches for different features?
@misterdjules: better not to centralize a lot of unrelated branches in a single repo.
@jasnell: do the same as I did for HTTP/2 repo, fork from nodejs/node into a new repo within github.com/nodejs.
@mhdawson: I’ll create another repo within github.com/nodejs.
Next steps:
- @mhdawson to commit draft to Node-EPs repo.
- @mhdawson to create another repo for ABI-stable work.
Other:
Modules meeting: http://doodle.com/poll/s4gcm28vmrdd3bqd
Q/A on public channels
None
Upcoming Meetings
- CTC: 2016-08-31
- TSC: 2016-08-25
- Benchmarking: Benchmarking#56
- Build:
- LTS:
- Diagnostics:
- Post-Mortem:
- API: