-
Notifications
You must be signed in to change notification settings - Fork 23
Default workflows part of all repositories #444
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
sheplu
wants to merge
1
commit into
master
Choose a base branch
from
rfc-define-core-workflows
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
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,79 @@ | ||
| # Standard default CI/CD workflows for Express.js repositories | ||
|
|
||
| ## Summary | ||
|
|
||
| This RFC proposes a standardized set of default GitHub Actions workflows that should exist in every repository within the Express.js organization. These workflows ensure consistent validation on code pushes, pull requests, and post-merge events, along with optional compatibility and maintenance checks. The goal is to guarantee consistent quality across projects while keeping workflows lightweight and maintainable. | ||
|
|
||
| ## Motivation | ||
|
|
||
| Currently, each repository uses its own GitHub Actions setup, or in some cases none at all. This creates: | ||
|
|
||
| - Inconsistent behavior across repositories | ||
| - Missed regressions in default branches | ||
| - Repeated maintenance effort | ||
| - Difficulty for new contributors to understand expectations | ||
| - No consistent testing against supported Node.js versions | ||
|
|
||
| By introducing standard workflows, we ensure: | ||
|
|
||
| - Consistent validation across repositories | ||
| - Predictable contributor experience | ||
| - Continued support for Node.js LTS policy | ||
| - Easier future migration to shared workflows | ||
|
|
||
| ## Detailed Explanation | ||
|
|
||
| ### Required Default Workflows | ||
|
|
||
| | Workflow | Trigger | Required | Description | | ||
| |----------|---------|----------|-------------| | ||
| | **1. Push Validation** | `on: push` to non-default branches | ✅ Yes | Run tests, lint, and basic validation for fast feedback during development. | | ||
| | **2. Pull Request Validation** | `on: pull_request` to `main`, `master`, or version branches | ✅ Yes | Ensures code is tested before merging. May include stricter checks than push validation. | | ||
| | **3. Post-Merge / Default Branch Check** | `on: push` to `main` or release branches | ✅ Yes | Ensures the default branch remains passing after merge and runs on the official Node.js LTS version(s). | | ||
| | **4. Manual Multi-Node.js Test** | `workflow_dispatch` | ✅ Yes (for core repos) | Allows manual execution of tests against multiple Node.js versions (e.g., 20, 22, 24) before releases or LTS changes. | | ||
| | **5. Scheduled Compatibility Check** | `schedule:` (optional) | Optional | Runs periodically to test against latest dependencies or Node.js nightly builds. | | ||
| | **6. Security / Dependency Audit** | Manual or scheduled | Optional | Runs `npm audit`, dependency checks, or license compliance. | | ||
| | **7. Release Workflow** | Tag or manual trigger | Optional | Builds and publishes packages (only for repositories that publish to npm). | | ||
|
|
||
| ### Guiding Principles | ||
|
|
||
| - Workflows should remain lightweight and fast. | ||
| - No mandatory CodeQL, coverage upload, or heavy tasks. | ||
| - All default workflows must be compatible with reusable workflows. | ||
| - Repositories may add extra workflows if needed but should not remove core workflows. | ||
|
|
||
| ## Rationale and Alternatives | ||
|
|
||
| | Approach | Pros | Cons | | ||
| |----------|------|------| | ||
| | **Proposed: Minimal Standard Workflows** | Balanced, lightweight, ensures consistency | Requires migration across repositories | | ||
| | Keep current undefined setup | No effort required | Inconsistency, higher risk of breakage | | ||
| | Enforce strict CI (coverage, audit, CodeQL, etc.) | Maximum protection | Too heavy for small modules, discouraging contributions | | ||
|
|
||
| ## Implementation | ||
|
|
||
| ### Affected Repositories | ||
|
|
||
| - All repositories under the Express.js organization that use or should use CI. | ||
|
|
||
| ### Steps | ||
|
|
||
| 1. Approve this RFC. | ||
| 2. Create example workflow templates in `.ci-workflows/workflows/`. | ||
| 3. Apply to core repositories: | ||
| - expressjs/express | ||
| - expressjs/router | ||
| - expressjs/body-parser | ||
| 4. Introduce shared workflows once this standard is normalized. | ||
| 5. Document requirements in `CONTRIBUTING.md`. | ||
|
|
||
| ## Prior Art | ||
|
|
||
| - GitHub recommends reusable organization workflows for consistency and maintenance. | ||
|
|
||
| ## Unresolved Questions | ||
|
|
||
| - Which Node.js versions must be tested by default? (e.g., 22 + 24 vs 18 + 20 + 22 + LTS) | ||
| - Should push validation run on all branches or exclude `main`? | ||
| - Naming convention for workflows (`ci.yml`, `test.yml`, `validate.yml`)? | ||
| - Should `npm audit` or dependency checks be mandatory or optional? | ||
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.
It definitely should be optional. All our dependencies are our own, and adding npm audit will only create noise in the workflow
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.
It’s not noise if it alerts about unaddressed CVEs tho