Skip to content
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: fix n-api addon build warnings #21808

Closed
wants to merge 1 commit into from

Conversation

kfarnung
Copy link
Contributor

@kfarnung kfarnung commented Jul 14, 2018

Fixed an MSVC warning on Windows:

  • test_general.c - Lossy conversion from int64 to double, explicitly
    casting to double resolved the warning
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@kfarnung kfarnung self-assigned this Jul 14, 2018
@kfarnung kfarnung requested a review from devsnek July 14, 2018 01:00
@nodejs-github-bot nodejs-github-bot added dont-land-on-v4.x node-api Issues and PRs related to the Node-API. test Issues and PRs related to the tests. labels Jul 14, 2018
@richardlau
Copy link
Member

Looks like the test_bigint.c changes are being taken care of in #21796.

@kfarnung
Copy link
Contributor Author

Duplicate of #21796

@kfarnung kfarnung marked this as a duplicate of #21796 Jul 16, 2018
@kfarnung kfarnung closed this Jul 16, 2018
@kfarnung kfarnung deleted the napiwarnings branch July 16, 2018 05:30
@kfarnung kfarnung restored the napiwarnings branch July 16, 2018 05:30
@kfarnung kfarnung deleted the napiwarnings branch July 16, 2018 05:30
@kfarnung kfarnung restored the napiwarnings branch July 16, 2018 05:30
@kfarnung kfarnung reopened this Jul 16, 2018
@kfarnung
Copy link
Contributor Author

I'll let #21796 land and rebase on those changes, the remaining one is purely to quiet a warning in MSVC.

Fixed an MSVC warning on Windows:
* test_general.c - Lossy conversion from int64 to double, explicitly
  casting to double resolved the warning
@kfarnung kfarnung removed the request for review from devsnek July 16, 2018 21:10
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@addaleax
Copy link
Member

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 18, 2018
@maclover7
Copy link
Contributor

@trivikr
Copy link
Member

trivikr commented Aug 3, 2018

@maclover7
Copy link
Contributor

Landed in f8d34b9

@maclover7 maclover7 closed this Aug 4, 2018
maclover7 pushed a commit that referenced this pull request Aug 4, 2018
Fixed an MSVC warning on Windows:
* test_general.c - Lossy conversion from int64 to double, explicitly
  casting to double resolved the warning

PR-URL: #21808
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Aug 6, 2018
Fixed an MSVC warning on Windows:
* test_general.c - Lossy conversion from int64 to double, explicitly
  casting to double resolved the warning

PR-URL: #21808
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. node-api Issues and PRs related to the Node-API. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants