|
| 1 | +# Node Foundation CTC Meeting 2016-08-03 |
| 2 | + |
| 3 | +## Links |
| 4 | + |
| 5 | +* **Audio Recording**: TBP |
| 6 | +* **GitHub Issue**: https://github.com/nodejs/node/issues/7948 |
| 7 | +* **Minutes Google Doc**: <https://docs.google.com/document/d/1ufR5dNuN3JLmFPvCvLYGRa98_-eqy5yY3zJuTnT60K8/edit#heading=h.ncvpl89y747j> |
| 8 | +* _Previous Minutes Google Doc: <https://docs.google.com/document/d/1rHxRb-ImjwFPOCOapgRDLdhqr4LJvie7eDNPOnDu2UA>_ |
| 9 | + |
| 10 | + |
| 11 | +## Present |
| 12 | + |
| 13 | +* Anna Henningsen @addaleax (observer) |
| 14 | +* Bradley Meck @bmeck (observer/GoDaddy/TC39) |
| 15 | +* Ben Noordhuis @bnoordhuis (CTC) |
| 16 | +* Сковорода Никита Андреевич @ChALkeR (CTC) |
| 17 | +* Colin Ihrig @cjihrig (CTC) |
| 18 | +* Evan Lucas @evanlucas (CTC) |
| 19 | +* Jeremiah Senkpiel @Fishrock123 (CTC) |
| 20 | +* James M Snell @jasnell (CTC) |
| 21 | +* Josh Gavant @joshgav (observer/Microsoft) |
| 22 | +* Michael Dawson @mhdawson (CTC) |
| 23 | +* Brian White @mscdex (CTC) |
| 24 | +* Ali Ijaz Sheikh @ofrobots (CTC) |
| 25 | +* Bert Belder @piscisaureus (CTC) |
| 26 | +* Saúl Ibarra Corretgé @saghul (observer) |
| 27 | +* Rich Trott @Trott (CTC) |
| 28 | + |
| 29 | + |
| 30 | +## Standup |
| 31 | + |
| 32 | +* Anna Henningsen @addaleax (observer) |
| 33 | + * Issues & PR review |
| 34 | +* Bradley Meck @bmeck (observer/GoDaddy/TC39) |
| 35 | + * Went to TC39 |
| 36 | + * Modules are going to take a different direction |
| 37 | +* Ben Noordhuis @bnoordhuis (CTC) |
| 38 | + * Nothing special. |
| 39 | +* Сковорода Никита Андреевич @ChALkeR (CTC) |
| 40 | + * Working on the npm dataset rebuilding tool. Some comments on issues and PRs as usual. |
| 41 | +* Colin Ihrig @cjihrig (CTC) |
| 42 | + * Was on vacation |
| 43 | + * Reviewing issues and PRs since I've been back |
| 44 | +* Evan Lucas @evanlucas (CTC) |
| 45 | + * A little cherry-picking to v6.x |
| 46 | + * Working on getting commit validator running for PRs |
| 47 | +* Jeremiah Senkpiel @Fishrock123 (CTC) |
| 48 | + * Mostly away, experimenting with nucleus-js. |
| 49 | +* James M Snell @jasnell (CTC) |
| 50 | + * Node Summit |
| 51 | + * Exploring the possibility of an HTTP/2 implementation in core |
| 52 | + * Continued evaluation of the WHATWG URL implementation |
| 53 | + * Foundation-y / TSC-y stuff |
| 54 | + * Reviewing PRs, catching up still from vacation |
| 55 | +* Josh Gavant @joshgav (observer/Microsoft) |
| 56 | + * internal stuff, vacation |
| 57 | +* Michael Dawson @mhdawson (CTC) |
| 58 | + * Node Summit/catching up on issues after Node Summit |
| 59 | + * Starting to add linuxOne release machine/jobs |
| 60 | + * Adding new AIX machine from osuosl |
| 61 | + * landed a few minutes PRs |
| 62 | +* Brian White @mscdex (CTC) |
| 63 | + * Commenting on issues/PRs. |
| 64 | +* Ali Ijaz Sheikh @ofrobots (CTC) |
| 65 | + * Node Summit & internal stuff. Spent rest of time shepherding some backports. |
| 66 | + * Planning on writing a proposal for managing V8 for LTS |
| 67 | +* Bert Belder @piscisaureus (CTC) |
| 68 | + * Commented on an issue. |
| 69 | +* Rich Trott @Trott (CTC) |
| 70 | + * CTC/governance documentation updates |
| 71 | + * Onboarding (danbev postponed but we’ll get there, now scheduling with fhinkel, additional nominees welcome) |
| 72 | + * fixed a flaky test, investigating others |
| 73 | + |
| 74 | +## Agenda |
| 75 | + |
| 76 | +Extracted from **ctc-agenda** labelled issues and pull requests from the **nodejs org** prior to the meeting. |
| 77 | + |
| 78 | +### nodejs/node |
| 79 | + |
| 80 | +* CTC membership nomination: @addaleax [#7607](https://github.com/nodejs/node/issues/7607) |
| 81 | + |
| 82 | +* Revert fs changes [#7846](https://github.com/nodejs/node/pull/7846) |
| 83 | + |
| 84 | +* [meta] realpath issues in v6 [#7726](https://github.com/nodejs/node/issues/7726) |
| 85 | + |
| 86 | +* v4.5.0 proposal [#7688](https://github.com/nodejs/node/pull/7688) |
| 87 | + |
| 88 | +* build: drop support for VS 2013 in v7 [#7484](https://github.com/nodejs/node/issues/7484) |
| 89 | + |
| 90 | +* http: don't inherit from Object.prototype [#6102](https://github.com/nodejs/node/pull/6102) |
| 91 | + |
| 92 | + |
| 93 | +### nodejs/node-eps |
| 94 | + |
| 95 | +* proposal: WHATWG URL standard implementation [#28](https://github.com/nodejs/node-eps/pull/28) |
| 96 | + |
| 97 | +### other |
| 98 | + |
| 99 | +* doc: @piscisaureus has stepped-down from the CTC [#7969](https://github.com/nodejs/node/pull/7969) |
| 100 | + |
| 101 | +## Previous Meeting |
| 102 | + |
| 103 | +### nodejs/node |
| 104 | + |
| 105 | +* Role of CTC in semver-major changes needs clarification [#7848](https://github.com/nodejs/node/issues/7848) |
| 106 | +* Revert fs changes [#7846](https://github.com/nodejs/node/pull/7846) |
| 107 | +* doc: add information about CTC quorum rules [#7813](https://github.com/nodejs/node/pull/7813) |
| 108 | +* meta: provide example activities [#7744](https://github.com/nodejs/node/pull/7744) |
| 109 | +* meta: realpath issues in v6 [#7726](https://github.com/nodejs/node/issues/7726) |
| 110 | +* v4.5.0 proposal [#7688](https://github.com/nodejs/node/pull/7688) |
| 111 | +* punycode: deprecate punycode module [#7552](https://github.com/nodejs/node/pull/7552) |
| 112 | +* Node 6 fs.realpath behavior changes [#7175](https://github.com/nodejs/node/issues/7175) |
| 113 | +* http: don't inherit from Object.prototype [#6102](https://github.com/nodejs/node/pull/6102) |
| 114 | +* Seek legal advice on LICENSE and copyright blocks in code [#3979](https://github.com/nodejs/node/issues/3979) |
| 115 | + |
| 116 | +### nodejs/post-mortem |
| 117 | + |
| 118 | +* Repositories to contribute collaboratively [#30](https://github.com/nodejs/post-mortem/issues/30) |
| 119 | + |
| 120 | +### nodejs/node-eps |
| 121 | + |
| 122 | +* proposal: WHATWG URL standard implementation [#28](https://github.com/nodejs/node-eps/pull/28) |
| 123 | + |
| 124 | +## Minutes |
| 125 | + |
| 126 | +### CTC membership nomination: @addaleax [#7607](https://github.com/nodejs/node/issues/7607) |
| 127 | + |
| 128 | +Unanimous `aye`. |
| 129 | + |
| 130 | +**Next steps**: @rvagg to merge. |
| 131 | + |
| 132 | +--- |
| 133 | + |
| 134 | +### Revert fs changes [#7846](https://github.com/nodejs/node/pull/7846) |
| 135 | + |
| 136 | +Reverts: |
| 137 | +https://github.com/nodejs/node/pull/7846 |
| 138 | +https://github.com/nodejs/node/pull/7950 |
| 139 | + |
| 140 | +Warn instead of throw when callback is omitted, as in v5: |
| 141 | +https://github.com/nodejs/node/pull/7897 |
| 142 | + |
| 143 | +@bnoordhuis: This change makes omitting the callback an immediate error. |
| 144 | + |
| 145 | +@trott: What do we need to do to get those PRs to land? |
| 146 | + |
| 147 | +@addaleax: Myles split off one commit to [#7950](https://github.com/nodejs/node/pull/7950). Not controversial, was requested by CTC last week. |
| 148 | +This must be landed prior to #7846. |
| 149 | + |
| 150 | +@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. |
| 151 | + |
| 152 | +@bnoordhuis: We need to print a deprecation warning [instead of throwing, when no callback]. |
| 153 | + |
| 154 | +@addaleax: That’s the plan after the reverts have landed. @thefourtheye plans to work on it. [see #7897]) |
| 155 | + |
| 156 | +@jasnell: process warning or deprecation warning? Will we use `util.printDeprecationWarning` or `process.emitWarning()`? |
| 157 | +Deprecation warning is semver-major, process warning is semver-minor or even patch. |
| 158 | + |
| 159 | +[It's `printDeprecationWarning`, see [here](https://github.com/thefourtheye/io.js/blob/8c65f7b6a253ab4e26ffe0de791dc41fcee92244/lib/fs.js#L48).] |
| 160 | + |
| 161 | +@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. |
| 162 | + |
| 163 | +@Fishrock123: Unbreaking a break and replacing with warning shouldn’t be semver-major. |
| 164 | + |
| 165 | +@trott: @jasnell’s PR on semver policies says it should be semver-major. |
| 166 | + |
| 167 | +@Fishrock123: Since we already changed it in the v6 transition let’s just change it to a deprecation. |
| 168 | + |
| 169 | +@trott: Let’s do the reverts (#7846, #7950), discuss deprecation warning separately in GH (#7897). |
| 170 | + |
| 171 | +OK with everybody. |
| 172 | + |
| 173 | +**Next steps**: Do the reverts (#7846, #7950). Continue discussion on throw -> warning in #7897. |
| 174 | + |
| 175 | +--- |
| 176 | + |
| 177 | +### [meta] realpath issues in v6 [#7726](https://github.com/nodejs/node/issues/7726) |
| 178 | + |
| 179 | +@trott: Last week we concluded that Anna, Trevor, or Alexis would move it forward. |
| 180 | + |
| 181 | +@trott: Just the two reverts that are blocking. |
| 182 | + |
| 183 | +@addaleax: Yes. |
| 184 | + |
| 185 | +@saghul: Old JS impl did not resolve subst’ed drives. New libuv impl does. A test looks for the new behavior. ] |
| 186 | + |
| 187 | +@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? |
| 188 | + |
| 189 | +@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. |
| 190 | + |
| 191 | +@piscisaureus: Path limit is 64k not likely to encounter. |
| 192 | + |
| 193 | +@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. |
| 194 | + |
| 195 | +@addaleax: Reverting to fs JS impl would return to old behavior. |
| 196 | + |
| 197 | +@trott: Submit a PR to remove that test, or move to known issues tests. |
| 198 | + |
| 199 | +@piscisaureus: Leave the test to track changes in the future. |
| 200 | + |
| 201 | +@Fishrock123: Key is to revert to an impl the ecosystem is depending on. Discuss this in another PR in GitHub. |
| 202 | + |
| 203 | +@jasnell: It’s a revert of the internal impl, not the changed public API [i.e. so we have to consider it now.] |
| 204 | + |
| 205 | +What about reverting the removal of the `cache` option? |
| 206 | + |
| 207 | +@addaleax: Not including the `cache` option now would allow additional/alternate improvements in the future. |
| 208 | + |
| 209 | +@bnoordhuis: Are these changes to land in v7? |
| 210 | + |
| 211 | +@jasnell: The idea is to get these into *v6* before it goes LTS. |
| 212 | + |
| 213 | +* The realpath change would land in v6. |
| 214 | +* The other changes (revert throwing error on callback) is only in master and would not land in v6. |
| 215 | +* Are the realpath changes dependent on the others? |
| 216 | +* Only Myles’ changes conflict with realpath [see previous item]. |
| 217 | + |
| 218 | +@jasnell: Do we have the steps lined up? |
| 219 | + |
| 220 | +@addaleax: Myles revert (#7846, #7950), then realpath revert. |
| 221 | + |
| 222 | +@? apply semver-major changes that were reverted, with deprecation warning. |
| 223 | + |
| 224 | +@chalker: Can we keep tests which were added? Revert removes some tests which aren’t actually related to changes. |
| 225 | + |
| 226 | +@piscisareus: Same as @saghul’s comment, and we agreed to keep the new tests. So yes we should keep them. |
| 227 | + |
| 228 | +**Next steps**: |
| 229 | + |
| 230 | +* Modify PR to keep tests related to new behavior for reference. |
| 231 | +* Apply Myles' reverts (#7846, #7950) |
| 232 | +* Apply `realpath` revert. |
| 233 | +* Discuss other items (e.g. throw -> deprecation, proper realpath for subst'ed drives, cache impl) in GH issues. |
| 234 | + |
| 235 | +--- |
| 236 | + |
| 237 | +### v4.5.0 proposal [#7688](https://github.com/nodejs/node/pull/7688) |
| 238 | + |
| 239 | +@trott: Please test the RCs. |
| 240 | + |
| 241 | +@mhdawson and @jasnell: Building at IBM and not encountered anything. |
| 242 | + |
| 243 | +--- |
| 244 | + |
| 245 | +### build: drop support for VS 2013 in v7 [#7484](https://github.com/nodejs/node/issues/7484) |
| 246 | + |
| 247 | +@bnoordhuis: Based on @joaocgreis’s comment we can move from 2013 without trouble. |
| 248 | + |
| 249 | +@joshgav: @joaocgreis tested against many modules and with CITGM and encountered no problems. |
| 250 | + |
| 251 | +@piscisaureus: Can we wait for semver-major? |
| 252 | + |
| 253 | +@joshgav: Then we’ll be stuck with 2013 through out v6 LTS. |
| 254 | + |
| 255 | +@piscisaureus: Should we make another issue for v6? |
| 256 | + |
| 257 | +@jasnell: Let’s get this landed in master and test it out, then make a decision on putting in v6 before October. |
| 258 | + |
| 259 | +@trott: Getting pretty close, don’t want to be switching just a month before. |
| 260 | + |
| 261 | +**Next steps**: |
| 262 | + |
| 263 | +* Merge to master and test. |
| 264 | +* Determine in September if we can apply in v6. |
| 265 | + |
| 266 | +--- |
| 267 | + |
| 268 | +### http: don't inherit from Object.prototype [#6102](https://github.com/nodejs/node/pull/6102) |
| 269 | + |
| 270 | +@chrisdickinson has concerns, @mscdex contacted. No response yet. |
| 271 | + |
| 272 | +@Fishrock123: Let’s remove `ctc-agenda` tag for next week. |
| 273 | + |
| 274 | +@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. |
| 275 | + |
| 276 | +@ofrobots: If we are going to change this we should have a plan for if/how we’ll get to maps. |
| 277 | + |
| 278 | +@Fishrock123: There are too many people currently depending on this being a regular object. |
| 279 | + |
| 280 | +@ofrobots: We shouldn’t break the ecosystem twice. We should target something with maps for v8 or longer timeframe. |
| 281 | + |
| 282 | +@Fishrock123: Motivation is ? |
| 283 | + |
| 284 | +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. |
| 285 | + |
| 286 | +@Fishrock123: We’ll need to provide both old and new APIs side by side, and that could be hard to do with maps. |
| 287 | + |
| 288 | +@ofrobots: We can delegate old API to a Proxy and add deprecation warnings on that handler. |
| 289 | + |
| 290 | +@?: Is Proxy performant enough? |
| 291 | + |
| 292 | +@ofrobots: For a deprecated path is it okay to take a performance hit? |
| 293 | + |
| 294 | +@evan: Okay with current change proposal (i.e. to not inherit from Object) as long as it’s considered semver-major. |
| 295 | + |
| 296 | +@jasnell will work on this. |
| 297 | + |
| 298 | +**Next steps**: Decide whether to merge this or wait for maps-based impl. |
| 299 | + |
| 300 | +--- |
| 301 | + |
| 302 | +### nodejs/node-eps proposal: WHATWG URL standard implementation [#28](https://github.com/nodejs/node-eps/pull/28) |
| 303 | + |
| 304 | +@trott: briefing now, longer discussion next week. |
| 305 | + |
| 306 | +@jasnell: Everyone keep reviewing it. Goal is to land as “experimental” in master, doesn’t have to go in v6. |
| 307 | + |
| 308 | +@trevnorris: -1 on global `URL` variable. |
| 309 | + |
| 310 | +@Fishrock123 also against new globals other than lang features. |
| 311 | + |
| 312 | +@jasnell: It’s a global in browsers. It can be removed, it’s in its own commit. |
| 313 | + |
| 314 | +**Next steps**: @nodejs/ctc review @jasnell's proposal. Discuss next week. |
| 315 | + |
| 316 | +--- |
| 317 | + |
| 318 | +### doc: @piscisaureus has stepped-down from the CTC [#7969](https://github.com/nodejs/node/pull/7969) |
| 319 | + |
| 320 | +@Fishrock123: submit PR to remove @piscisaureus and/or move to Collaborator. |
| 321 | + |
| 322 | +--- |
| 323 | + |
| 324 | +## Q/A on public fora |
| 325 | + |
| 326 | +No questions. |
| 327 | + |
| 328 | +## Upcoming Meetings |
| 329 | + |
| 330 | +* CTC: 2016-08-03 |
| 331 | +* TSC: 2016-07-28 |
| 332 | +* Build: 2016-08-07 |
| 333 | +* LTS: 2016-07-25 |
| 334 | +* Diagnostics: Sept |
| 335 | +* Post-Mortem: August |
| 336 | +* API: August |
0 commit comments