-
Notifications
You must be signed in to change notification settings - Fork 18
Add policy to align engines fields with ci #289
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
Open
UlisesGascon
wants to merge
2
commits into
master
Choose a base branch
from
adr/engines
base: master
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 all commits
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
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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,70 @@ | ||
# ADR 001: Aligning `engines` Field in `package.json` with CI Version Support | ||
|
||
## Status | ||
Proposed | ||
|
||
## Submitters | ||
- Ulises Gascón (@UlisesGascon) | ||
|
||
## Decision Owners | ||
- Express TC (@expressjs/express-tc) | ||
|
||
## Context | ||
The issue of maintaining consistency between the `engines` field in `package.json` and the versions tested in CI has been raised. Several packages have dropped support for Node.js versions prior to v18, but this change has not been consistently applied across all packages. | ||
|
||
**Why do we need this decision?** | ||
Aligning the `engines` field with CI ensures that we explicitly define the minimum Node.js version supported for each release. This minimizes the risk of unintentional support for older versions that are not tested in CI, thus reducing maintenance overhead and unexpected bug reports. | ||
|
||
**What problem does it solve or avoid?** | ||
This decision will help avoid introducing breaking changes unintentionally, streamline user expectations, and ensure that end users are aware of the minimum Node.js version they should use. | ||
|
||
**Are there any existing issues/discussions/pull requests related to this?** | ||
- [Discussion: Using engines in the package.json #286](https://github.com/expressjs/discussions/issues/286) | ||
- [Missing engines Field Update for Node.js Version Support in v2.0.0 pillarjs/finalhandler#64](https://github.com/pillarjs/finalhandler/issues/64) | ||
|
||
## Decision | ||
We will establish a policy that requires the `engines` field in `package.json` to match the Node.js versions defined in our CI configuration. This policy will apply to all packages within the Express ecosystem. | ||
|
||
**What will be done?** | ||
- Update the `engines` field in `package.json` for each package to reflect the minimum Node.js version supported by the corresponding CI configuration. | ||
- Include the `engines` fields in all the projects, so it will clarify what is supported for the final user | ||
|
||
**What will not be done?** | ||
- We will not support versions of Node.js that are not explicitly defined in the `engines` field or tested in CI. | ||
|
||
## Rationale | ||
|
||
**Alternatives Considered:** | ||
- **Alternative 1:** Keep the `engines` field as-is and update only on a need basis. | ||
- **Reason for rejection:** This could lead to confusion among users and contributors regarding the supported Node.js versions, resulting in unintended support for older versions. | ||
- **Alternative 2:** Remove the `engines` field altogether and rely solely on documentation for version support. | ||
- **Reason for rejection:** The `engines` field provides a built-in way to notify users of version requirements, making it an effective tool for communicating support boundaries. | ||
|
||
**Pros and Cons**: | ||
- **Pros**: | ||
- Ensures clarity for users regarding the supported Node.js versions. | ||
- Reduces the risk of unintended support and compatibility issues. | ||
- Aligns with best practices for maintaining Node.js packages. | ||
- **Cons**: | ||
- Adjusting the `engines` field could be considered a breaking change, potentially impacting users on unsupported versions. | ||
|
||
**Why is this decision the best option?** | ||
This decision creates a clear alignment between our CI-tested versions and the `engines` field, providing transparency for users and consistency across the project. It balances the need for modern support with a clear boundary for legacy versions. | ||
|
||
## Consequences | ||
|
||
**Positive Impact**: | ||
- Users will have a clear understanding of the supported Node.js versions, reducing confusion and unexpected bug reports. | ||
- It allows the team to focus development on supported versions, leading to a more efficient development process. | ||
|
||
**Negative Impact**: | ||
- Users on older Node.js versions may be forced to upgrade sooner than expected. | ||
|
||
**Mitigations**: | ||
- Communicate this policy change clearly in release notes and documentation. | ||
|
||
## References | ||
- [NPM Documentation about `engines` field](https://docs.npmjs.com/cli/v10/configuring-npm/package-json#engines) | ||
|
||
## Changelog | ||
- **[2024-10-22]**: @UlisesGascon - Initial draft of ADR for aligning `engines` with CI versions. |
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.
Uh oh!
There was an error while loading. Please reload this page.