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

watch: batch file restarts #51992

Merged
merged 1 commit into from
Mar 9, 2024
Merged

watch: batch file restarts #51992

merged 1 commit into from
Mar 9, 2024

Conversation

MoLow
Copy link
Member

@MoLow MoLow commented Mar 6, 2024

Fixes #51954

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Mar 6, 2024
@MoLow MoLow mentioned this pull request Mar 6, 2024
Closed
@matthieusieben
Copy link
Contributor

matthieusieben commented Mar 7, 2024 •
edited
Loading

I am so disappointed.

I spent several hours posting an issue and went so far as to provide not one but 4 different ways to fix it. In response, I get no feed back and @MoLow starts taking things out of my hands and does the thing himself.

Great 👍

@MoLow
Copy link
Member Author

MoLow commented Mar 7, 2024

@matthieusieben I did not mean to take things out of your hands, I simply opened PRS just to propose a solution I thought was better to the problem.
I am happy with closing this PR and discussing your proposed solutions (that I think are incorrect) in #51954. I probably should have done that prior to opening this PR
I want to emphesise again there was no intent to leave you out of things, and that is why I added you as a co-author in 57ac709

@MoLow MoLow closed this Mar 7, 2024
@matthieusieben
Copy link
Contributor

Look, this solution is clearly supperior to #51971 so just keep it. I just wished you helped me figure things out instead of doing it yourself.

@MoLow MoLow mentioned this pull request Mar 7, 2024
Closed
This was referenced Mar 7, 2024
Closed
Closed
@MoLow
Copy link
Member Author

MoLow commented Mar 7, 2024

@matthieusieben I intend to re-open and add you as a co-author. please confirm you are ok with being added as a co-author

@matthieusieben
Copy link
Contributor

I confirm. Thank you

@MoLow @matthieusieben
watch: batch file restarts
a38e4a8 
Co-authored-by: Matthieu <matthieusieben@users.noreply.github.com>
@MoLow MoLow reopened this Mar 7, 2024
@MoLow MoLow force-pushed the watch-mode-lock-restart branch from 897f07c to a38e4a8 Compare March 7, 2024 16:21
@MoLow MoLow requested review from Linkgoron, benjamingr and atlowChemi and removed request for benjamingr and atlowChemi March 7, 2024 16:21
@atlowChemi atlowChemi added request-ci Add this label to start a Jenkins CI on a PR. watch-mode Issues and PRs related to watch mode labels Mar 7, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 7, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@MoLow MoLow added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 7, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@matthieusieben matthieusieben mentioned this pull request Mar 8, 2024
Closed
@nodejs-github-bot
Copy link
Collaborator

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

@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 9, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 9, 2024
@nodejs-github-bot nodejs-github-bot merged commit 257f322 into nodejs:main Mar 9, 2024
61 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 257f322

@MoLow MoLow deleted the watch-mode-lock-restart branch March 9, 2024 22:03
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 26, 2024
@MoLow @matthieusieben @rdw-msft
watch: batch file restarts
c90864e 
Co-authored-by: Matthieu <matthieusieben@users.noreply.github.com>
PR-URL: nodejs#51992
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Apr 15, 2024
Merged
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@MoLow @matthieusieben @marco-ippolito
watch: batch file restarts
508e968 
Co-authored-by: Matthieu <matthieusieben@users.noreply.github.com>
PR-URL: #51992
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@marco-ippolito marco-ippolito mentioned this pull request May 2, 2024
Merged
jcbhmr pushed a commit to jcbhmr/node that referenced this pull request May 15, 2024
@MoLow @matthieusieben @jcbhmr
watch: batch file restarts
83a5668 
Co-authored-by: Matthieu <matthieusieben@users.noreply.github.com>
PR-URL: nodejs#51992
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atlowChemi atlowChemi atlowChemi approved these changes

@benjamingr benjamingr benjamingr approved these changes

@Linkgoron Linkgoron Linkgoron approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. watch-mode Issues and PRs related to watch mode
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

watch should debounce the restart
6 participants
@MoLow @matthieusieben @nodejs-github-bot @Linkgoron @benjamingr @atlowChemi