Skip to content

src: remove ignore GCC -Wcast-function-type for v8 #35768

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
wants to merge 1 commit into from
Closed

src: remove ignore GCC -Wcast-function-type for v8 #35768

wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Oct 23, 2020 •
edited
Loading

This reverts Commit 3ff2aec ("src: ignore GCC -Wcast-function-type for v8.h") and Commit
2462a2c ("src: fix ignore GCC -Wcast-function-type for older compilers") as this has
now been included in the V8 version being used.

Refs:
v8/v8@f08cbfd

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 23, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@danbev danbev mentioned this pull request Oct 23, 2020
Closed
2 tasks
@codecov-io
Copy link

Codecov Report

Merging #35768 into master will decrease coverage by 8.49%.
The diff coverage is 73.80%.

@@            Coverage Diff             @@
##           master   #35768      +/-   ##
==========================================
- Coverage   96.40%   87.90%   -8.50%     
==========================================
  Files         223      477     +254     
  Lines       73685   113090   +39405     
  Branches        0    24627   +24627     
==========================================
+ Hits        71038    99417   +28379     
- Misses       2647     7962    +5315     
- Partials        0     5711    +5711
Impacted Files Coverage Δ
src/base_object-inl.h 78.12% <ø> (ø)
src/node.h 93.33% <ø> (ø)
src/util.h 85.24% <ø> (ø)
src/inspector_profiler.cc 76.17% <69.44%> (ø)
lib/v8.js 99.65% <100.00%> (-0.35%) ⬇️
src/inspector_profiler.h 86.95% <100.00%> (ø)
lib/internal/idna.js 55.55% <0.00%> (-11.12%) ⬇️
lib/internal/blocklist.js 88.70% <0.00%> (-10.49%) ⬇️
lib/internal/crypto/mac.js 73.45% <0.00%> (-9.48%) ⬇️
lib/internal/modules/esm/get_format.js 84.72% <0.00%> (-8.34%) ⬇️
... and 395 more

Trott
Trott approved these changes Oct 23, 2020
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM. Might be worth double-checking the commit message text to make sure it's what you intended.

@danbev
src: remove ignore GCC -Wcast-function-type for v8
3ae98ac 
This reverts Commit 3ff2aec ("src:
ignore GCC -Wcast-function-type for v8.h") and Commit
2462a2c ("src: fix ignore GCC
-Wcast-function-type for older compilers") as this has
now been included in the V8 version being used.

Refs:
v8/v8@f08cbfd
@danbev danbev force-pushed the remove-gcc-cast-function-type2 branch from f2f788f to 3ae98ac Compare October 23, 2020 13:22
@danbev
Copy link
Contributor Author

danbev commented Oct 23, 2020

Might be worth double-checking the commit message text to make sure it's what you intended

Thanks for pointing that out, it was not very clear.

@nodejs-github-bot
Copy link
Collaborator

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

@danbev
Copy link
Contributor Author

danbev commented Oct 26, 2020 •
edited
Loading

Re-run of failing /node-test-commit-linux-containered ✔️
Re-run of failing node-test-commit-windows-fanned ✔️

danbev added a commit that referenced this pull request Oct 26, 2020
@danbev
src: remove ignore GCC -Wcast-function-type for v8
9ff25b1 
This reverts Commit 3ff2aec ("src:
ignore GCC -Wcast-function-type for v8.h") and Commit
2462a2c ("src: fix ignore GCC
-Wcast-function-type for older compilers") as this has
now been included in the V8 version being used.

PR-URL: #35768
Refs: v8/v8@f08cbfd
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@danbev
Copy link
Contributor Author

danbev commented Oct 26, 2020

Landed in 9ff25b1.

@danbev danbev closed this Oct 26, 2020
@danbev danbev deleted the remove-gcc-cast-function-type2 branch October 26, 2020 11:45
targos pushed a commit that referenced this pull request Nov 3, 2020
@danbev @targos
src: remove ignore GCC -Wcast-function-type for v8
4128eef 
This reverts Commit 3ff2aec ("src:
ignore GCC -Wcast-function-type for v8.h") and Commit
2462a2c ("src: fix ignore GCC
-Wcast-function-type for older compilers") as this has
now been included in the V8 version being used.

PR-URL: #35768
Refs: v8/v8@f08cbfd
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@targos targos mentioned this pull request Nov 3, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott approved these changes

@targos targos targos approved these changes

@JungMinu JungMinu JungMinu approved these changes

+1 more reviewer

@devnexen devnexen devnexen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@danbev @nodejs-github-bot @codecov-io @Trott @targos @devnexen @JungMinu