Skip to content

src: move windows specific fns to _WIN32 #57951

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
Merged
merged 1 commit into from
Apr 22, 2025
Merged

src: move windows specific fns to _WIN32 #57951

merged 1 commit into from
Apr 22, 2025

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Apr 20, 2025

We don't need to define these functions for non-windows systems. We can just put it behind a _WIN32 defined check.

@anonrig anonrig requested a review from jasnell April 20, 2025 16:01
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Apr 20, 2025
@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Apr 20, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 20, 2025
@nodejs-github-bot
Copy link
Collaborator

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

@codecov Codecov
Copy link

codecov bot commented Apr 20, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.26%. Comparing base (7102ea1) to head (68b6787).
Report is 16 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #57951      +/-   ##
==========================================
- Coverage   90.27%   90.26%   -0.01%     
==========================================
  Files         630      630              
  Lines      186115   186124       +9     
  Branches    36467    36475       +8     
==========================================
+ Hits       168010   168011       +1     
- Misses      10973    10975       +2     
- Partials     7132     7138       +6
Files with missing lines Coverage Δ
src/util-inl.h 80.98% <ø> (ø)
src/util.h 91.22% <ø> (ø)

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@anonrig anonrig force-pushed the yagiz/avoid-defining-is-windows-batch-file branch from f6b6f23 to 68b6787 Compare April 20, 2025 22:44
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@anonrig anonrig requested review from mcollina and aduh95 April 21, 2025 00:08
@anonrig anonrig added the review wanted PRs that need reviews. label Apr 21, 2025
@anonrig anonrig added commit-queue Add this label to land a pull request using GitHub Actions. and removed review wanted PRs that need reviews. labels Apr 21, 2025
@aduh95
Copy link
Contributor

aduh95 commented Apr 22, 2025

CI: https://ci.nodejs.org/job/node-test-commit/79356/

@aduh95 aduh95 removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 22, 2025
@aduh95 aduh95 merged commit 1720b18 into nodejs:main Apr 22, 2025
58 of 59 checks passed
@aduh95
Copy link
Contributor

aduh95 commented Apr 22, 2025

Landed in 1720b18

RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@anonrig @RafaelGSS
src: move windows specific fns to _WIN32
ca25de9 
PR-URL: #57951
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
@RafaelGSS RafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
@anonrig @RafaelGSS
src: move windows specific fns to _WIN32
2877207 
PR-URL: #57951
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@anonrig @aduh95
src: move windows specific fns to _WIN32
727ce28 
PR-URL: #57951
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
RafaelGSS pushed a commit that referenced this pull request May 14, 2025
@anonrig @RafaelGSS
src: move windows specific fns to _WIN32
0d6ceed 
PR-URL: #57951
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
aduh95 pushed a commit that referenced this pull request May 16, 2025
@anonrig @aduh95
src: move windows specific fns to _WIN32
5cad6d8 
PR-URL: #57951
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@anonrig @aduh95
src: move windows specific fns to _WIN32
ddfe283 
PR-URL: #57951
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@anonrig @aduh95
src: move windows specific fns to _WIN32
fd4b227 
PR-URL: #57951
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@anonrig @aduh95
src: move windows specific fns to _WIN32
0abd76c 
PR-URL: #57951
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
aduh95 pushed a commit that referenced this pull request May 18, 2025
@anonrig @aduh95
src: move windows specific fns to _WIN32
41a11c8 
PR-URL: #57951
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
@github-actions github-actions bot mentioned this pull request May 19, 2025
Merged
aduh95 pushed a commit that referenced this pull request May 19, 2025
@anonrig @aduh95
src: move windows specific fns to _WIN32
aee45e2 
PR-URL: #57951
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@StefanStojanovic StefanStojanovic StefanStojanovic approved these changes

@pmarchini pmarchini pmarchini approved these changes

@mcollina mcollina Awaiting requested review from mcollina

@aduh95 aduh95 Awaiting requested review from aduh95

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@anonrig @nodejs-github-bot @aduh95 @jasnell @lpinca @StefanStojanovic @pmarchini