PR-URL: https://github.com/nodejs/node/pull/7980 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
12 KiB
Node Foundation CTC Meeting 2016-08-03
Links
- Audio Recording: TBP
- GitHub Issue: https://github.com/nodejs/node/issues/7948
- Minutes Google Doc: https://docs.google.com/document/d/1ufR5dNuN3JLmFPvCvLYGRa98_-eqy5yY3zJuTnT60K8/edit#heading=h.ncvpl89y747j
- Previous Minutes Google Doc: https://docs.google.com/document/d/1rHxRb-ImjwFPOCOapgRDLdhqr4LJvie7eDNPOnDu2UA
Present
- Anna Henningsen @addaleax (observer)
- Bradley Meck @bmeck (observer/GoDaddy/TC39)
- Ben Noordhuis @bnoordhuis (CTC)
- Сковорода Никита Андреевич @ChALkeR (CTC)
- Colin Ihrig @cjihrig (CTC)
- Evan Lucas @evanlucas (CTC)
- Jeremiah Senkpiel @Fishrock123 (CTC)
- James M Snell @jasnell (CTC)
- Josh Gavant @joshgav (observer/Microsoft)
- Michael Dawson @mhdawson (CTC)
- Brian White @mscdex (CTC)
- Ali Ijaz Sheikh @ofrobots (CTC)
- Bert Belder @piscisaureus (CTC)
- Saúl Ibarra Corretgé @saghul (observer)
- Rich Trott @Trott (CTC)
Standup
- Anna Henningsen @addaleax (observer)
- Issues & PR review
- Bradley Meck @bmeck (observer/GoDaddy/TC39)
- Went to TC39
- Modules are going to take a different direction
- Ben Noordhuis @bnoordhuis (CTC)
- Nothing special.
- Сковорода Никита Андреевич @ChALkeR (CTC)
- Working on the npm dataset rebuilding tool. Some comments on issues and PRs as usual.
- Colin Ihrig @cjihrig (CTC)
- Was on vacation
- Reviewing issues and PRs since I've been back
- Evan Lucas @evanlucas (CTC)
- A little cherry-picking to v6.x
- Working on getting commit validator running for PRs
- Jeremiah Senkpiel @Fishrock123 (CTC)
- Mostly away, experimenting with nucleus-js.
- James M Snell @jasnell (CTC)
- Node Summit
- Exploring the possibility of an HTTP/2 implementation in core
- Continued evaluation of the WHATWG URL implementation
- Foundation-y / TSC-y stuff
- Reviewing PRs, catching up still from vacation
- Josh Gavant @joshgav (observer/Microsoft)
- internal stuff, vacation
- Michael Dawson @mhdawson (CTC)
- Node Summit/catching up on issues after Node Summit
- Starting to add linuxOne release machine/jobs
- Adding new AIX machine from osuosl
- landed a few minutes PRs
- Brian White @mscdex (CTC)
- Commenting on issues/PRs.
- Ali Ijaz Sheikh @ofrobots (CTC)
- Node Summit & internal stuff. Spent rest of time shepherding some backports.
- Planning on writing a proposal for managing V8 for LTS
- Bert Belder @piscisaureus (CTC)
- Commented on an issue.
- Rich Trott @Trott (CTC)
- CTC/governance documentation updates
- Onboarding (danbev postponed but we’ll get there, now scheduling with fhinkel, additional nominees welcome)
- fixed a flaky test, investigating others
Agenda
Extracted from ctc-agenda labelled issues and pull requests from the nodejs org prior to the meeting.
nodejs/node
-
CTC membership nomination: @addaleax #7607
-
Revert fs changes #7846
-
[meta] realpath issues in v6 #7726
-
v4.5.0 proposal #7688
-
build: drop support for VS 2013 in v7 #7484
-
http: don't inherit from Object.prototype #6102
nodejs/node-eps
- proposal: WHATWG URL standard implementation #28
other
- doc: @piscisaureus has stepped-down from the CTC #7969
Previous Meeting
nodejs/node
- Role of CTC in semver-major changes needs clarification #7848
- Revert fs changes #7846
- doc: add information about CTC quorum rules #7813
- meta: provide example activities #7744
- meta: realpath issues in v6 #7726
- v4.5.0 proposal #7688
- punycode: deprecate punycode module #7552
- Node 6 fs.realpath behavior changes #7175
- http: don't inherit from Object.prototype #6102
- Seek legal advice on LICENSE and copyright blocks in code #3979
nodejs/post-mortem
- Repositories to contribute collaboratively #30
nodejs/node-eps
- proposal: WHATWG URL standard implementation #28
Minutes
CTC membership nomination: @addaleax #7607
Unanimous aye
.
Next steps: @rvagg to merge.
Revert fs changes #7846
Reverts: https://github.com/nodejs/node/pull/7846 https://github.com/nodejs/node/pull/7950
Warn instead of throw when callback is omitted, as in v5: https://github.com/nodejs/node/pull/7897
@bnoordhuis: This change makes omitting the callback an immediate error.
@trott: What do we need to do to get those PRs to land?
@addaleax: Myles split off one commit to #7950. Not controversial, was requested by CTC last week. This must be landed prior to #7846.
@jasnell: Would like to see a CI and CITGM run and some additional testing to make sure we have exactly the right set of reverts. @jasnell will start it.
@bnoordhuis: We need to print a deprecation warning [instead of throwing, when no callback].
@addaleax: That’s the plan after the reverts have landed. @thefourtheye plans to work on it. [see #7897])
@jasnell: process warning or deprecation warning? Will we use util.printDeprecationWarning
or process.emitWarning()
?
Deprecation warning is semver-major, process warning is semver-minor or even patch.
[It's printDeprecationWarning
, see here.]
@addaleax: PR to print warning already opened. Could be used instead of reverting, but we agreed to revert last week and it blocks the realpath revert.
@Fishrock123: Unbreaking a break and replacing with warning shouldn’t be semver-major.
@trott: @jasnell’s PR on semver policies says it should be semver-major.
@Fishrock123: Since we already changed it in the v6 transition let’s just change it to a deprecation.
@trott: Let’s do the reverts (#7846, #7950), discuss deprecation warning separately in GH (#7897).
OK with everybody.
Next steps: Do the reverts (#7846, #7950). Continue discussion on throw -> warning in #7897.
[meta] realpath issues in v6 #7726
@trott: Last week we concluded that Anna, Trevor, or Alexis would move it forward.
@trott: Just the two reverts that are blocking.
@addaleax: Yes.
@saghul: Old JS impl did not resolve subst’ed drives. New libuv impl does. A test looks for the new behavior. ]
@saghul: Some people are relying on the old JS impl behavior to shorten paths on Windows. What are the right semantics? Should subst
ed drives resolve to original path or to shortened path?
@piscisaureus: The whole reason we use realpath in module
is to avoid a module being loaded multiple times via multiple symlinked paths. So the goal should be to always resolve to the true path.
@piscisaureus: Path limit is 64k not likely to encounter.
@saghul: There’s a test for the new code which expects realpath on a subst’ed drive to give the original path. We need to revert this test for reverted behavior.
@addaleax: Reverting to fs JS impl would return to old behavior.
@trott: Submit a PR to remove that test, or move to known issues tests.
@piscisaureus: Leave the test to track changes in the future.
@Fishrock123: Key is to revert to an impl the ecosystem is depending on. Discuss this in another PR in GitHub.
@jasnell: It’s a revert of the internal impl, not the changed public API [i.e. so we have to consider it now.]
What about reverting the removal of the cache
option?
@addaleax: Not including the cache
option now would allow additional/alternate improvements in the future.
@bnoordhuis: Are these changes to land in v7?
@jasnell: The idea is to get these into v6 before it goes LTS.
- The realpath change would land in v6.
- The other changes (revert throwing error on callback) is only in master and would not land in v6.
- Are the realpath changes dependent on the others?
- Only Myles’ changes conflict with realpath [see previous item].
@jasnell: Do we have the steps lined up?
@addaleax: Myles revert (#7846, #7950), then realpath revert.
@? apply semver-major changes that were reverted, with deprecation warning.
@chalker: Can we keep tests which were added? Revert removes some tests which aren’t actually related to changes.
@piscisareus: Same as @saghul’s comment, and we agreed to keep the new tests. So yes we should keep them.
Next steps:
- Modify PR to keep tests related to new behavior for reference.
- Apply Myles' reverts (#7846, #7950)
- Apply
realpath
revert. - Discuss other items (e.g. throw -> deprecation, proper realpath for subst'ed drives, cache impl) in GH issues.
v4.5.0 proposal #7688
@trott: Please test the RCs.
@mhdawson and @jasnell: Building at IBM and not encountered anything.
build: drop support for VS 2013 in v7 #7484
@bnoordhuis: Based on @joaocgreis’s comment we can move from 2013 without trouble.
@joshgav: @joaocgreis tested against many modules and with CITGM and encountered no problems.
@piscisaureus: Can we wait for semver-major?
@joshgav: Then we’ll be stuck with 2013 through out v6 LTS.
@piscisaureus: Should we make another issue for v6?
@jasnell: Let’s get this landed in master and test it out, then make a decision on putting in v6 before October.
@trott: Getting pretty close, don’t want to be switching just a month before.
Next steps:
- Merge to master and test.
- Determine in September if we can apply in v6.
http: don't inherit from Object.prototype #6102
@chrisdickinson has concerns, @mscdex contacted. No response yet.
@Fishrock123: Let’s remove ctc-agenda
tag for next week.
@jasnell: Plenty of discussion, heard from Doug Wilson that it’s a positive change. Would rather not hold up waiting for a response. Would be best to land before v7.
@ofrobots: If we are going to change this we should have a plan for if/how we’ll get to maps.
@Fishrock123: There are too many people currently depending on this being a regular object.
@ofrobots: We shouldn’t break the ecosystem twice. We should target something with maps for v8 or longer timeframe.
@Fishrock123: Motivation is ?
Some feel we should wait for maps before making a breaking change. Jeremiah feels we should make an effective change now and not wait for an unknown future.
@Fishrock123: We’ll need to provide both old and new APIs side by side, and that could be hard to do with maps.
@ofrobots: We can delegate old API to a Proxy and add deprecation warnings on that handler.
@?: Is Proxy performant enough?
@ofrobots: For a deprecated path is it okay to take a performance hit?
@evan: Okay with current change proposal (i.e. to not inherit from Object) as long as it’s considered semver-major.
@jasnell will work on this.
Next steps: Decide whether to merge this or wait for maps-based impl.
nodejs/node-eps proposal: WHATWG URL standard implementation #28
@trott: briefing now, longer discussion next week.
@jasnell: Everyone keep reviewing it. Goal is to land as “experimental” in master, doesn’t have to go in v6.
@trevnorris: -1 on global URL
variable.
@Fishrock123 also against new globals other than lang features.
@jasnell: It’s a global in browsers. It can be removed, it’s in its own commit.
Next steps: @nodejs/ctc review @jasnell's proposal. Discuss next week.
doc: @piscisaureus has stepped-down from the CTC #7969
@Fishrock123: submit PR to remove @piscisaureus and/or move to Collaborator.
Q/A on public fora
No questions.
Upcoming Meetings
- CTC: 2016-08-03
- TSC: 2016-07-28
- Build: 2016-08-07
- LTS: 2016-07-25
- Diagnostics: Sept
- Post-Mortem: August
- API: August