-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: update weak module for gc tests #7014
Conversation
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning.
I'm not about to review a +9,391 −279 diff so rubber-stamp LGTM. Meta: The gc tests are not run regularly. How about we either add them to the CI or we scrap them? I personally don't find them that valuable but I'm interested in what others think. |
Oy, yeah, that diff is harsh lol. rubber-stamp LGTM here also if CI is possible. Also agree with @bnoordhuis in that if we're not running these regularly, they're not of very much use to keep around. |
Maybe we should run these in citgm? cc @thealphanerd ? |
@Fishrock123 The changes are in |
@Fishrock123 Oh, wait, maybe you're just suggesting that |
@Trott I'm suggesting that maybe we should put it in a separate repo and include it in the citgm tests. |
CI: https://ci.nodejs.org/job/node-test-pull-request/2883/ EDIT: One Pi buildbot explosion and one Pi inexplicably hung during a |
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning. PR-URL: nodejs#7014 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Landed in 0d7f313 |
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning. PR-URL: #7014 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning. PR-URL: #7014 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning. PR-URL: #7014 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning. PR-URL: #7014 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning. PR-URL: #7014 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning. PR-URL: #7014 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning. PR-URL: #7014 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning. PR-URL: #7014 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning. PR-URL: #7014 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Checklist
Affected core subsystem(s)
test
Description of change
Previous version of weak used for gc tests emitted a warning on OS X.
Updating to current version eliminates warning.
R=@bnoordhuis?