Skip to content
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

build: don't clean obj.target directory if it doesn't exist #54337

Merged
merged 2 commits into from
Aug 24, 2024
Merged

build: don't clean obj.target directory if it doesn't exist #54337

merged 2 commits into from
Aug 24, 2024
+7 −3

Conversation

joyeecheung
Copy link
Member

This can happen to ninja builds, which would then throw an error instead of being a noop if the command is executed.

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Aug 12, 2024
@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 12, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 12, 2024
@nodejs-github-bot
Copy link
Collaborator

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

legendecas
legendecas reviewed Aug 12, 2024
View reviewed changes
Copy link
Member

@legendecas legendecas left a comment •
edited
Loading

Choose a reason for hiding this comment

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

make coverage-clean uses the same command and will fail. Would you mind updating it as well?

@codecov Codecov
Copy link

codecov bot commented Aug 12, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.31%. Comparing base (59da1df) to head (f0cbf62).
Report is 66 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #54337      +/-   ##
==========================================
+ Coverage   87.08%   87.31%   +0.23%     
==========================================
  Files         648      648              
  Lines      182341   182321      -20     
  Branches    34977    34966      -11     
==========================================
+ Hits       158789   159202     +413     
+ Misses      16827    16388     -439     
- Partials     6725     6731       +6

see 55 files with indirect coverage changes

@joyeecheung
build: don't clean obj.target directory if it doesn't exist
b7221e7 
This can happen to ninja builds, which would then throw an
error instead of being a noop if the command is executed.
@joyeecheung
fixup! build: don't clean obj.target directory if it doesn't exist
f0cbf62
@joyeecheung
Copy link
Member Author

Updated coverage-clean

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 19, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 19, 2024
@nodejs-github-bot
Copy link
Collaborator

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

legendecas
legendecas approved these changes Aug 19, 2024
View reviewed changes
Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

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

Thanks!

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 21, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 21, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@joyeecheung joyeecheung added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Aug 24, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 24, 2024
@nodejs-github-bot nodejs-github-bot merged commit e272cfb into nodejs:main Aug 24, 2024
61 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in e272cfb

RafaelGSS pushed a commit that referenced this pull request Aug 25, 2024
@joyeecheung @RafaelGSS
build: don't clean obj.target directory if it doesn't exist
a3f4e56 
This can happen to ninja builds, which would then throw an
error instead of being a noop if the command is executed.

PR-URL: #54337
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Aug 25, 2024
Merged
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
@joyeecheung @RafaelGSS
build: don't clean obj.target directory if it doesn't exist
467ac3a 
This can happen to ninja builds, which would then throw an
error instead of being a noop if the command is executed.

PR-URL: #54337
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca approved these changes

@anonrig anonrig anonrig approved these changes

@legendecas legendecas legendecas approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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.
5 participants
@joyeecheung @nodejs-github-bot @lpinca @anonrig @legendecas