Node.js Foundation Core Technical Committee (CTC) Meeting 2017-01-25 #63
Comments
|
#10599, #10155 and #10135 can be dropped from the agenda unless there have been new developments. (#10135 I'm certain about because I didn't do anything.) |
|
Maybe consider including nodejs/node#10853? |
|
Apologies, but it's quite likely I'll be unable to attend this week. I'll be back on schedule next week. Thanks! |
|
To clarify why #10282 is on the agenda, see nodejs/node#10282 (comment).
|
@Trott I had previously submitted a PR for that (nodejs/node#2315) I personally think we need to figure out what we want to do with assert. We obviously can't remove it, so why not make it actually work properly. Just my opinion though. |
|
I don't have a problem with unfreezing assert. Our story that assert is only for testing node itself isn't very convincing. We could simply That said, I still think it's a good idea to encourage people to use third-party modules. Assertion testing is not a core responsibility - the plethora of testing libs shows it doesn't need to be in core - and we have to be extremely conservative. Any API design mistake is a multi-year project to fix and even bug fixes will be semver-major most of the time. |
|
I've been considering the possibility that |
Sure, we can remove it. We would do this:
Whether we would prefer to have endless debates when people (rightly) point out its not a very good library for writing unit tests, or to just push it into npmjs where it can be developed and semvered and the ecosystem can flourish (or just stop using it and move to one of the other actively developed assertion libs) is the question. We have a bit of a horror of removing things from node, but IMO, that kind of horror should be balanced with a stronger aversion to adding things to node. Lets not let "easy to get in - hard to get out" be our motto! |
|
@jasnell the problem with vendoring into core as a long-term strategy is it makes semver-major improvements very painful. With npm modules semver-major is NBD, no one gets hurt, though they probably will want to update sometime (or not, if the old one works and there code is stable). When we vendor into core, we force the semver changes on people updating node, even when they update node because they need some critical-to-them feature, but are now forced to eat the price of updating to the latest assert. The assert usage is probably not even in their code, but in their deps, so updating can be quite painful. Its a decent temporary strategy, but not long-term, in my opinion. |
|
On the assert thing: The smallest thing we could do to get things unstuck in my opinion is to agree that modifying the assert module to deal with things like Symbols, Maps, Sets, etc. does not constitute changing the API (assuming no function signature changes). So making those sorts of fixes does not require unlocking the API etc. (The changes may be semver-major out of caution.) |
|
Let's add a quick discussion about nodejs/node#10187 (node-inspect) to the meeting. |
|
It looks like I'll probably still be in transit by the time the call starts. Apologies if I don't join. |
|
suggestion: should we add a list of items tagged seems like we usually have a few items like that each week, nodejs/node#10970 is an example of one I might add this week, want to put it on your radar but probably don't need to discuss further today. |
|
notes in #64 |
|
Sorry, my alarm failed me 😢 |
assert.deepEqual and assert.deepStrictEqual currently return true for any pair of Maps and Sets regardless of content. This patch adds support in deepEqual and deepStrictEqual to verify the contents of Maps and Sets. Unfortunately because there's no way to pairwise fetch set values or map values which are equivalent but not reference-equal, this change currently only supports reference equality checking in set values and map key values. Equivalence checking could be done, but it would be an O(n^2) operation, and worse, it would get slower exponentially if maps and sets were nested. Note that this change breaks compatibility with previous versions of deepEqual and deepStrictEqual if consumers were depending on all maps and sets to be seen as equivalent. The old behaviour was never documented, but nevertheless there are certainly some tests out there which depend on it. Support has stalled because the assert API was frozen, but was recently unfrozen in CTC#63 Fixes: nodejs#2309 Refs: tape-testing/tape#342 Refs: nodejs#2315 Refs: nodejs/CTC#63
assert.deepEqual and assert.deepStrictEqual currently return true for any pair of Maps and Sets regardless of content. This patch adds support in deepEqual and deepStrictEqual to verify the contents of Maps and Sets. Deeo equivalence checking is currently an O(n^2) operation, and worse, it gets slower exponentially if maps and sets were nested. Note that this change breaks compatibility with previous versions of deepEqual and deepStrictEqual if consumers were depending on all maps and sets to be seen as equivalent. The old behaviour was never documented, but nevertheless there are certainly some tests out there which depend on it. Support has stalled because the assert API was frozen, but was recently unfrozen in CTC#63. --- Later squashed in: This change updates the checks for deep equality checking on Map and Set to check all set values / all map keys to see if any of them match the expected result. This change is much slower, but based on the conversation in the pull request its probably the right approach. Fixes: #2309 Refs: tape-testing/tape#342 Refs: #2315 Refs: nodejs/CTC#63 PR-URL: #12142 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Time
UTC Wed 25-Jan-2017 16:00 (04:00 PM):
Or in your local time:
Links
Agenda
Extracted from ctc-agenda labelled issues and pull requests from the nodejs org prior to the meeting.
nodejs/node
nodejs/TSC
nodejs/node-eps
Invited
Notes
The agenda comes from issues labelled with
ctc-agendaacross all of the repositories in the nodejs org. Please label any additional issues that should be on the agenda before the meeting starts.Joining the meeting
Uberconference; participants should have the link & numbers, contact me if you don't.
Public participation
We stream our conference call straight to YouTube so anyone can listen to it live, it should start playing at https://www.youtube.com/c/nodejs+foundation/live when we turn it on. There's usually a short cat-herding time at the start of the meeting and then occasionally we have some quick private business to attend to before we can start recording & streaming. So be patient and it should show up.
Many of us will be on IRC in #node-dev on Freenode if you'd like to interact, we have a Q/A session scheduled at the end of the meeting if you'd like us to discuss anything in particular. @nodejs/collaborators in particular if there's anything you need from the CTC that's not worth putting on as a separate agenda item, this is a good place for it.
The text was updated successfully, but these errors were encountered: