Skip to content

test: fix n-api addon build warnings#21808

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.

Sign up for GitHub

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

Jump to bottom
Closed
kfarnung wants to merge 1 commit into from
Closed

test: fix n-api addon build warnings #21808

kfarnung wants to merge 1 commit into from
+1 −1

Conversation

@kfarnung
Copy link
Contributor

@kfarnungkfarnung commented Jul 14, 2018•
edited
Loading

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
Collaborator

nodejs-github-bot commented Jul 14, 2018

@kfarnung build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/272/pipeline

@kfarnungkfarnung 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
Member

richardlau commented Jul 14, 2018

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

@kfarnung
Copy link
ContributorAuthor

kfarnung commented Jul 16, 2018

Duplicate of#21796

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

kfarnung commented Jul 16, 2018

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

@kfarnung
test: fix n-api addon build warnings
8d12e3a
Fixed an MSVC warning on Windows: * test_general.c - Lossy conversion from int64 to double, explicitly casting to double resolved the warning
@kfarnungkfarnung removed the request for review from devsnekJuly 16, 2018 21:10
mhdawson
mhdawson approved these changes Jul 17, 2018
View reviewed changes
Copy link
Member

@mhdawsonmhdawson 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 commented Jul 18, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/15934/

@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
Contributor

maclover7 commented Jul 30, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/16083/

@trivikr
Copy link
Member

trivikr commented Aug 3, 2018

Resumed CI: https://ci.nodejs.org/job/node-test-pull-request/16173/

@maclover7
Copy link
Contributor

maclover7 commented Aug 4, 2018

Landed in f8d34b9

maclover7 pushed a commit that referenced this pull request Aug 4, 2018
@kfarnung@maclover7
test: fix n-api addon build warnings
f8d34b9
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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
targos pushed a commit that referenced this pull request Aug 6, 2018
@kfarnung@targos
test: fix n-api addon build warnings
d38ccaa
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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@rvaggrvagg mentioned this pull request Aug 13, 2018
Merged
This was referenced Aug 16, 2018
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes

@addaleaxaddaleaxaddaleax approved these changes

@cjihrigcjihrigcjihrig approved these changes

@mhdawsonmhdawsonmhdawson approved these changes

Assignees

@kfarnungkfarnung

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

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