Skip to content

Conversation

@kvakil
Copy link
Contributor

@kvakil kvakil commented Sep 24, 2022

Fix various warnings which show up when compiling with gcc. For the most part this doesn't change any semantics -- the only user-facing change is a bug fix in Printer::Stringify caused by a missing return.

Fix various warnings which show up when compiling with gcc. For the most
part this doesn't change any semantics -- the only user-facing change is
a bug fix in `Printer::Stringify` caused by a missing `return`.
@codecov-commenter
Copy link

codecov-commenter commented Sep 24, 2022

Codecov Report

Base: 73.54% // Head: 73.18% // Decreases project coverage by -0.36% ⚠️

Coverage data is based on head (cadcb62) compared to base (d91aa40).
Patch coverage: 77.77% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #415      +/-   ##
==========================================
- Coverage   73.54%   73.18%   -0.37%     
==========================================
  Files          34       34              
  Lines        5072     5071       -1     
==========================================
- Hits         3730     3711      -19     
- Misses       1342     1360      +18     
Impacted Files Coverage Δ
src/llv8.h 74.13% <ø> (ø)
src/printer.cc 75.29% <33.33%> (+0.10%) ⬆️
src/llscan.cc 60.56% <100.00%> (-1.86%) ⬇️
src/llv8.cc 71.37% <100.00%> (-0.14%) ⬇️
src/llnode_module.cc 87.79% <0.00%> (-0.59%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

kvakil added a commit to kvakil/llnode that referenced this pull request Sep 24, 2022
This test failed a couple of times in nodejs#415 and nodejs#417, so let's skip it
for now.
kvakil added a commit to kvakil/llnode that referenced this pull request Sep 24, 2022
This test failed a couple of times in nodejs#415, nodejs#417 and nodejs#418, so let's skip
it for now.
kvakil added a commit to kvakil/llnode that referenced this pull request Sep 24, 2022
This test failed a couple of times in nodejs#415, nodejs#417 and nodejs#418, so let's skip
it for now.
@No9 No9 self-requested a review September 24, 2022 07:25
@No9
Copy link
Member

No9 commented Sep 24, 2022

LGTM - I'm rerunning the the flaky tests and then I'll land it.
I'll also review the other PRs and then look to do a release.

@No9 No9 merged commit 1e111aa into nodejs:main Sep 24, 2022
No9 pushed a commit that referenced this pull request Sep 25, 2022
This test failed a couple of times in #415, #417 and #418, so let's skip
it for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants