Skip to content

test: fix n-api addon build warnings - #21808

Closed
kfarnung wants to merge 1 commit into
nodejs:masterfrom
kfarnung:napiwarnings
Closed

test: fix n-api addon build warnings#21808
kfarnung wants to merge 1 commit into
nodejs:masterfrom
kfarnung:napiwarnings

Conversation

@kfarnung

@kfarnungkfarnung commented Jul 14, 2018

Copy link
Copy Markdown
Contributor

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

@kfarnungkfarnung self-assigned this Jul 14, 2018
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@kfarnung
kfarnung requested a review from devsnekJuly 14, 2018 01:00
@nodejs-github-botnodejs-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
Copy Markdown
Member

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

@kfarnung

Copy link
Copy Markdown
ContributorAuthor

Duplicate of#21796

@kfarnungkfarnung marked this as a duplicate of #21796Jul 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
@kfarnungkfarnung reopened this Jul 16, 2018
@kfarnung

Copy link
Copy Markdown
ContributorAuthor

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 devsnekJuly 16, 2018 21:10

@mhdawsonmhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@addaleax

Copy link
Copy Markdown
Member

@addaleaxaddaleax 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
Copy Markdown
Contributor

@trivikr

Copy link
Copy Markdown
Member

@maclover7

Copy link
Copy Markdown
Contributor

Landed in f8d34b9

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>
@rvaggrvagg mentioned this pull request Aug 13, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.node-apiIssues and PRs related to the Node-API.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@kfarnung@nodejs-github-bot@richardlau@addaleax@maclover7@trivikr@jasnell@cjihrig@mhdawson@Trott