Revert "console: use consolePropAttributes for k-bind properties in c… by danbev · Pull Request #26943 · nodejs/node (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation12 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
…onstructor"
This reverts commit ed5e69d.
The failed test case it flaky and Travis is only an indicator about things working or not. I don't think that this should be reverted / that it's related.
@BridgeAR Ah good to know that it's flakey, thanks!
I'm a little confused about this issue as it does seem to be consistent on master when I run it locally and after reverting it did not fail (though I did not run it more than twice).
I'll let others chime in as well and see what people think is the best path forward.
We can find out by reverting it and see if it flakes again. If it flakes, revert the revert.
@danbev it's also reproducible for me. I am puzzled that it seems to make the test fail reliable. @addaleax PTAL.
To get the test at least pass again on most systems, it's probably fine to revert it for now. It just does not seem to be the actual issue to me.
I am puzzled that it seems to make the test fail reliable.
@gireeshpunathil was asking for a core dump in #26401, if anyone could accommodate...
tl;dr I'm +1 on reverting to get master stable again, then use the previous flaky code to chase down the underlying bug.
danbev added a commit that referenced this pull request
…onstructor"
This reverts commit ed5e69d.
PR-URL: #26943 Refs: #26850 (comment) Reviewed-By: Joyee Cheung joyeec9h3@gmail.com Reviewed-By: Colin Ihrig cjihrig@gmail.com Reviewed-By: Refael Ackermann refack@gmail.com
This did not fix the issue as described here: #26401 (comment)
This revert should probably be reverted again.
This was referenced
Apr 23, 2019
This was referenced
Apr 23, 2019
BridgeAR added a commit to BridgeAR/node that referenced this pull request
This is a reland of nodejs#26850. It was speculatively reverted but it turned out that this did not cause any trouble.
PR-URL: nodejs#27352 Refs: nodejs#26943 Refs: nodejs#26850 Reviewed-By: Daniel Bevenius daniel.bevenius@gmail.com Reviewed-By: Refael Ackermann refack@gmail.com Reviewed-By: Rich Trott rtrott@gmail.com
targos pushed a commit that referenced this pull request
This is a reland of #26850. It was speculatively reverted but it turned out that this did not cause any trouble.
PR-URL: #27352 Refs: #26943 Refs: #26850 Reviewed-By: Daniel Bevenius daniel.bevenius@gmail.com Reviewed-By: Refael Ackermann refack@gmail.com Reviewed-By: Rich Trott rtrott@gmail.com
Labels
Issues and PRs related to the console subsystem.
PRs that do not need to wait for 48 hours to land.