Skip to content

Conversation

@aakashamy777
Copy link

No description provided.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/config

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Dec 3, 2025
Copy link
Member

@marco-ippolito marco-ippolito left a comment

Choose a reason for hiding this comment

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

This change removes the test and watch namespaces completely.
This is not what #60904 is about, it should only remove the test- prefix from keys.
The commit breaks many tests, run make test. Also it does not respect the commit guidelines, should be "lib: remove prefix from node.config.json namespaces" or something like that
Moreover I believe it should be an alias and not forbid the test- completely.

@marco-ippolito marco-ippolito added the config Issues or PRs related to the config subsystem label Dec 3, 2025
@aakashamy777
Copy link
Author

aakashamy777 commented Dec 3, 2025

okay i will go through the issue and guidelines again, will update the codebase

@IlyasShabi
Copy link
Contributor

In addition of what Marco said, please add tests and PR description

}
}
},
"test": {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we supposed to remove the prefix instead of the whole namespace no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config Issues or PRs related to the config subsystem needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants