GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — watch: reload changes in contents of --env-file by marekpiechut · Pull Request #54109 · nodejs/node

Conversation

@marekpiechut

@marekpiechut marekpiechut commented Jul 29, 2024

Copy link
Copy Markdown
Contributor

Make sure we watch and reload on env file changes.
Ignore env file in parent process, so child process can reload current vars when we recreate it.

Fixes: #54001

I've noticed now that it's probably a duplicate of #54033.

Feel free to close or reuse tests from here in the other PR if you plan on merging it.
It was a nice small bug to get feet wet with Node internals anyway. 👋

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss, daeyeon, HBSPS, theoludwig, and OliverJAsh reacted with thumbs up emoji
All reactions
  • 👍 5 reactions
@marekpiechut
watch: reload changes in contents of --env-file
cdeff79 
Make sure we watch and reload on env file changes.

Ignore env file in parent process, so child process can reload
current vars when we recreate it.

Fixes: nodejs#54001
@nodejs-github-bot

nodejs-github-bot commented Jul 29, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/startup
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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 Jul 29, 2024
anonrig
anonrig approved these changes Jul 30, 2024
View reviewed changes
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jul 31, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jul 31, 2024
@nodejs-github-bot

nodejs-github-bot commented Jul 31, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60759/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@codecov

codecov Bot commented Jul 31, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.07%. Comparing base (cf9a814) to head (a8eecc8).
Report is 540 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #54109   +/-   ##
=======================================
  Coverage   87.07%   87.07%           
=======================================
  Files         643      643           
  Lines      181590   181576   -14     
  Branches    34893    34892    -1     
=======================================
- Hits       158114   158109    -5     
+ Misses      16754    16746    -8     
+ Partials     6722     6721    -1
Files with missing lines Coverage Δ
src/node.cc 74.03% <100.00%> (ø)

... and 31 files with indirect coverage changes
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@marekpiechut
watch: reload changes in contents of --env-file (fix liner issues)
a8eecc8
@marekpiechut

marekpiechut commented Jul 31, 2024

Copy link
Copy Markdown
Contributor Author

Fixed linter issues.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
MoLow
MoLow approved these changes Aug 2, 2024
View reviewed changes
@MoLow MoLow added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 2, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 2, 2024
@nodejs-github-bot

nodejs-github-bot commented Aug 2, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60833/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@marekpiechut

marekpiechut commented Aug 4, 2024

Copy link
Copy Markdown
Contributor Author

I see there are test failures in the latest check. I tried to look into them, but they all look weird. There are some unrelated test failures and build issues on Windows, etc.

@MoLow @anonrig could you give some advice how to continue with that?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoLow

MoLow commented Aug 4, 2024

Copy link
Copy Markdown
Member

none of those seem related
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Aug 4, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60873/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Aug 5, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60879/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Aug 5, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60880/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoLow MoLow added commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Aug 5, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 5, 2024
@nodejs-github-bot
nodejs-github-bot merged commit a816688 into nodejs:main Aug 5, 2024
@nodejs-github-bot

nodejs-github-bot commented Aug 5, 2024

Copy link
Copy Markdown
Collaborator

Landed in a816688
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@marekpiechut
marekpiechut deleted the watch-env-file branch August 5, 2024 11:25
@tniessen

tniessen commented Aug 5, 2024

Copy link
Copy Markdown
Member

Are there no NODE_OPTIONS that should affect the parent process?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoLow

MoLow commented Aug 5, 2024

Copy link
Copy Markdown
Member

@tniessen I am not sure what you mean? can you elaborate?
passing --env-file will be correctly forwarded to inner process regardless of if it is used with NODE_OPTIONS or without
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
targos pushed a commit that referenced this pull request Aug 14, 2024
@marekpiechut @targos
watch: reload changes in contents of --env-file
4d8b53e 
Make sure we watch and reload on env file changes.

Ignore env file in parent process, so child process can reload
current vars when we recreate it.

Fixes: #54001
PR-URL: #54109
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Aug 19, 2024
v22.7.0 proposal #54452
Merged
@targos targos added the dont-land-on-v20.x label Sep 21, 2024
@gorankarlic gorankarlic mentioned this pull request Nov 26, 2024
--watch-path also watches --env file directory tree in >=22.7 #55913
Closed
@dchenk

dchenk commented Jan 12, 2025

Copy link
Copy Markdown

This change didn't cover the --env-file-if-exists flag. Please see #49148 (comment)
BoscoDomingo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anonrig anonrig anonrig approved these changes
@MoLow MoLow MoLow approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash PRs the Commit Queue should land as one squashed commit. 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.

--env-file watch

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@marekpiechut @nodejs-github-bot @MoLow @tniessen @dchenk @anonrig @targos
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.