-
Notifications
You must be signed in to change notification settings - Fork 10
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.
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
feat(semver-major): Upgrade package to Node.js v23 #54
Open
avivkeller
wants to merge
2
commits into
nodejs:main
Choose a base branch
from
avivkeller:v23
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
|
||
test | ||
.github | ||
avivkeller marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
".": "4.0.0" | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
# Maintenance Guide for the Package | ||
|
||
This guide outlines the procedures necessary for maintaining this package, ensuring its functionality and compatibility with newer versions of Node.js. The goal is to streamline the maintenance process until a majority of users can transition to the latest versions, paving the way for eventual deprecation of this library. | ||
|
||
## Overview | ||
|
||
Maintaining this library involves updating specific internal files and ensuring that all references are correctly modified. The steps below provide a clear pathway for effective package maintenance. | ||
|
||
## Maintenance Steps | ||
|
||
### 1. Identify Files for Update | ||
Start by identifying the internal files that require updates. These files are typically located in the `lib/internal/` directory. For example, `lib/internal/test_runner/runner.js` is one file that may need attention. | ||
|
||
### 2. Update File Contents | ||
- Replace the entire contents of the identified file with the updated version from your reference source. Ensure you use the correct version that corresponds to the changes made in Node.js internals. | ||
|
||
### 3. Modify Require Statements | ||
- After replacing the file contents, locate all instances of the following pattern in the file: | ||
```javascript | ||
require('internal/...'); | ||
``` | ||
- Update these instances to the new syntax: | ||
avivkeller marked this conversation as resolved.
Show resolved
Hide resolved
|
||
```javascript | ||
require('#internal/...'); | ||
``` | ||
|
||
### 4. Add Necessary Imports | ||
- If the updated file requires specific bindings, include the following line at the top of the file: | ||
```javascript | ||
const { primordials, internalBinding } = require('#lib/bootstrap'); | ||
``` | ||
|
||
### 5. Follow Special Comments | ||
- Pay close attention to any comments formatted as `/* NOTE(Author): ... */` within the files. These notes may contain essential guidelines or considerations regarding the code. Adhere to any instructions provided in these comments during the update process. | ||
|
||
### 6. Implement Polyfills as Needed | ||
- When updating the library, you may encounter new features that require polyfilling if they are not present in the library. Add the minimal amount of code necessary for functionality. For instance, in `lib/internal/options`, only implement parsing for the options that are actually needed. | ||
|
||
## Final Steps | ||
|
||
- After completing the updates, conduct thorough testing of the package to ensure all functionality works as expected with the new changes. | ||
- Document any significant modifications made during the update process to maintain a clear history for future reference. | ||
|
||
If you have any questions about this document, it was written by @RedYetiDev. |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's bring back the matrix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.This package isn't compatible with v20, or with v16. It's specifically meant for v18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably be aiming for support on all supported versions of Node.js. If there's something that makes it work only for v18 but not v20, something's probably wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look into it, but we really don't need to support v20. v20 has most features that this aims to replicate