Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

async_wrap: add a missing case to test-async-wrap-throw-no-init #8198

Merged
merged 1 commit into from
Aug 25, 2016

Conversation

yorkie
Copy link
Contributor

@yorkie yorkie commented Aug 20, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

async_wrap, test

Description of change

This covers this line in "src/async-wrap.cc". /R= @trevnorris

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Aug 20, 2016
@jasnell
Copy link
Member

jasnell commented Aug 21, 2016

@jasnell
Copy link
Member

jasnell commented Aug 21, 2016

LGTM if CI is green.

@yorkie
Copy link
Contributor Author

yorkie commented Aug 22, 2016

The CI seems to fail on the following errors:

@yorkie
Copy link
Contributor Author

yorkie commented Aug 22, 2016

@jasnell The arm-fanned issue seems to be tried to fix at #8203, but another seems fresh, so I opened #8219 to track that.

@trevnorris
Copy link
Contributor

LGTM

@yorkie
Copy link
Contributor Author

yorkie commented Aug 23, 2016

Re-running a new ci: https://ci.nodejs.org/job/node-test-pull-request/3815/

PR-URL: nodejs#8198
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@yorkie yorkie force-pushed the test-async-wrap-throw-no-init branch from c49a070 to 10b3f13 Compare August 25, 2016 19:54
@yorkie yorkie merged commit 10b3f13 into nodejs:master Aug 25, 2016
@yorkie yorkie deleted the test-async-wrap-throw-no-init branch August 25, 2016 19:56
@yorkie
Copy link
Contributor Author

yorkie commented Aug 25, 2016

Landed at 10b3f13, thanks for reviews :)

@yorkie
Copy link
Contributor Author

yorkie commented Aug 25, 2016

Needs help with the CI failure, the CI seems to fail on AIX, here is the log of failed build. I remember that this CI#3815 was green but now it has failure. /cc @nodejs/build

@Fishrock123 Fishrock123 mentioned this pull request Sep 6, 2016
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
PR-URL: nodejs#8198
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Sep 9, 2016
PR-URL: #8198
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 30, 2016
PR-URL: #8198
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@MylesBorins
Copy link
Contributor

I've landed this on v4.x-staging. @trevnorris @yorkie let me know if it should not have landed

MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
PR-URL: #8198
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
PR-URL: #8198
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
PR-URL: #8198
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 26, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants