FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: fix n-api addon build warnings by kfarnung · Pull Request #21808 · nodejs/node · GitHub

/ node Public

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

Closed
kfarnung wants to merge 1 commit into
nodejs:masterfrom
kfarnung:napiwarnings
ConversationCommits (1)ChecksFiles changed
Closed

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

Conversation

kfarnung commented Jul 14, 2018 •
edited
Loading

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

kfarnung self-assigned this Jul 14, 2018

nodejs-github-bot commented Jul 14, 2018

Copy link
Copy Markdown
Collaborator

@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

kfarnung requested a review from devsnek July 14, 2018 01:00
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 commented Jul 14, 2018

Copy link
Copy Markdown
Member

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

kfarnung commented Jul 16, 2018

Copy link
Copy Markdown
Contributor Author

Duplicate of #21796

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

kfarnung commented Jul 16, 2018

Copy link
Copy Markdown
Contributor Author

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

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
kfarnung removed the request for review from devsnek July 16, 2018 21:10
mhdawson approved these changes Jul 17, 2018
View reviewed changes

mhdawson 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

addaleax commented Jul 18, 2018

Copy link
Copy Markdown
Member

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

addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jul 18, 2018

maclover7 commented Jul 30, 2018

Copy link
Copy Markdown
Contributor

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

trivikr commented Aug 3, 2018

Copy link
Copy Markdown
Member

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

maclover7 commented Aug 4, 2018

Copy link
Copy Markdown
Contributor

Landed in f8d34b9

maclover7 closed this Aug 4, 2018
maclover7 pushed a commit that referenced this pull request Aug 4, 2018
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 <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
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 <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>
rvagg mentioned this pull request Aug 13, 2018
Merged
This was referenced Aug 16, 2018
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
addaleax addaleax approved these changes
cjihrig cjihrig approved these changes
+1 more reviewer
mhdawson mhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

kfarnung

Labels

author ready PRs that have at least one approval, no outstanding review comments, 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

Back | FazBrowse Home | New Git URL