-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Add GitHub Actions CI, linting, and project setup #251
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
Conversation
This commit introduces a comprehensive CI setup for the project, along with essential linting and configuration files. Key changes include: - **GitHub Actions Workflows:** - `ci.yml`: Configures a complete continuous integration pipeline including TypeScript tests, action testing, and build steps. - `check-dist.yml`: Adds a workflow to ensure the `dist/` directory contains the expected transpiled code. - `codeql-analysis.yml`: Integrates CodeQL for static analysis. - `licensed.yml`: Sets up dependency license checking using the Licensed tool. - `linter.yml`: Utilizes Super-linter for comprehensive codebase linting. - **Configuration Files:** - `.env.example`: Provides an example environment file. - `.eslinlint.yml`, `.markdown-lint.yml`, `.prettierignore`, `.prettierrc.yml`, `.yaml-lint.yml`: Standard configuration files for ESLint, markdownlint, Prettier, and yamllint, ensuring code consistency. - `.vscode/launch.json`, `.vscode/mcp.json`, `.vscode/settings.json`: VS Code configuration for debugging and workspace settings. - `.licensed.yml`, `.node-version`: Configuration for the Licensed tool and Node.js version. - `actionlint.yml`: Configuration for Actionlint. - `rollup.config.ts`: Rollup configuration for bundling. - `tsconfig.base.json`, `tsconfig.eslint.json`, `tsconfig.json`: TypeScript configuration files for base settings, ESLint, and the main project. - **Codebase Structuring:** - `.gitignore`: Updated to ignore built artifacts and local environment files. - Test files (`__tests__`) refactored for consistency. - `action.yml`: Updated with refined descriptions and defaults. These changes establish a robust foundation for code quality, automated testing, and streamlined development workflows.
Caution Review failedThe pull request is closed. WalkthroughAdds CI/workflow configs, linting and license tooling, Rollup/ESM packaging, Node 24 pin, TypeScript/ESLint config changes, license manifests, repository/editor configs, a .env example, and multiple import/formatting edits across tests and source files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant GH as GitHub
participant CI as CI (ci.yml)
participant Node as actions/setup-node
participant NPM as npm
Dev->>GH: Push / Open PR to main
GH->>CI: Trigger CI jobs
CI->>Node: setup Node (from .node-version)
CI->>NPM: npm ci
CI->>CI: format:check → lint → ci-test → build
alt push (not PR)
CI->>GH: git-auto-commit-action commits updated dist/
end
sequenceDiagram
autonumber
participant PR as Pull Request
participant WF as Check Transpiled JavaScript workflow
participant Node as actions/setup-node
participant NPM as npm
participant Git as git
PR->>WF: on push/PR
WF->>Node: setup Node
WF->>NPM: npm ci
WF->>NPM: npm run bundle (build dist/)
WF->>WF: verify dist/ exists
WF->>Git: git diff dist/ vs committed
alt differences found
WF-->>PR: fail + print diff
WF->>GH: upload-artifact dist/
else
WF-->>PR: success
end
sequenceDiagram
autonumber
participant Src as Repository
participant Index as `src/index.ts`
participant Main as `src/main.ts`
Src->>Index: module load
Index->>Main: import run()
Index->>Main: invoke run()
Note over Main: run() is now exported (explicit call via index)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (17)
Comment |
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
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.
Actionable comments posted: 8
🧹 Nitpick comments (2)
action.yml (1)
9-17
: Use folded block scalars for wrapped descriptionsSwitching to multi-line single-quoted scalars inserts literal newlines (and leading spaces) into the input descriptions. GitHub Action metadata expects simple single-line strings here, so those newlines will leak into the rendered help text. Please either keep the original one-line strings or use a folded block (
>
) to wrap without changing the content.rollup.config.ts (1)
7-16
: Consider whether to externalize dependencies.The current configuration bundles all dependencies into the output. For GitHub Actions, this is often the desired behavior (single-file distribution). However, if you want to externalize certain packages (e.g., large or peer dependencies), you can add an
external
option.Example to externalize all node_modules:
const config = { input: 'src/index.ts', + external: (id) => /node_modules/.test(id), output: {
Only apply this if you have a specific need to avoid bundling certain dependencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
badges/coverage.svg
is excluded by!**/*.svg
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (53)
.env.example
(1 hunks).gitattributes
(1 hunks).github/codeql/codeql-config.yml
(1 hunks).github/dependabot.yml
(2 hunks).github/workflows/check-dist.yml
(1 hunks).github/workflows/ci.yml
(1 hunks).github/workflows/codeql-analysis.yml
(1 hunks).github/workflows/codeql.yml
(0 hunks).github/workflows/licensed.yml
(1 hunks).github/workflows/linter.yml
(1 hunks).github/workflows/nodejs.yml
(0 hunks).gitignore
(1 hunks).licensed.yml
(1 hunks).licenses/npm/@actions/core.dep.yml
(1 hunks).licenses/npm/@actions/exec.dep.yml
(1 hunks).licenses/npm/@actions/http-client.dep.yml
(1 hunks).licenses/npm/@actions/io.dep.yml
(1 hunks).licenses/npm/@fastify/busboy.dep.yml
(1 hunks).licenses/npm/tunnel.dep.yml
(1 hunks).licenses/npm/undici.dep.yml
(1 hunks).markdown-lint.yml
(1 hunks).node-version
(1 hunks).nvmrc
(0 hunks).prettierignore
(1 hunks).prettierrc.json
(0 hunks).prettierrc.yml
(1 hunks).vscode/launch.json
(1 hunks).vscode/mcp.json
(1 hunks).vscode/settings.json
(1 hunks).yaml-lint.yml
(1 hunks)README.md
(2 hunks)__tests__/custom.test.ts
(1 hunks)__tests__/grumphp.test.ts
(1 hunks)__tests__/phpcs.test.ts
(1 hunks)__tests__/phplint.test.ts
(2 hunks)__tests__/phpmd.test.ts
(1 hunks)__tests__/phpstan.test.ts
(1 hunks)action.yml
(1 hunks)actionlint.yml
(1 hunks)eslint.config.mjs
(1 hunks)jest.config.js
(1 hunks)package.json
(2 hunks)rollup.config.ts
(1 hunks)src/checks/custom.ts
(1 hunks)src/checks/grumphp.ts
(1 hunks)src/checks/phpcs.ts
(1 hunks)src/checks/phplint.ts
(1 hunks)src/checks/phpmd.ts
(1 hunks)src/checks/phpstan.ts
(1 hunks)src/main.ts
(2 hunks)tsconfig.base.json
(1 hunks)tsconfig.eslint.json
(1 hunks)tsconfig.json
(1 hunks)
💤 Files with no reviewable changes (4)
- .prettierrc.json
- .nvmrc
- .github/workflows/nodejs.yml
- .github/workflows/codeql.yml
🧰 Additional context used
🧬 Code graph analysis (1)
__tests__/phplint.test.ts (1)
src/checks/phplint.ts (1)
phplint
(3-31)
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
60-60: input "milliseconds" is not defined in action "DrupalQA" defined at "./". available inputs are "checks", "php-version", "registry", "web-root"
(action)
64-64: property "time" is not defined in object type {}
(expression)
🪛 GitHub Actions: Continuous Integration
eslint.config.mjs
[error] 1-1: ESLint failed to load configuration: Cannot find package 'globals' imported from eslint.config.mjs.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint Codebase
🔇 Additional comments (12)
__tests__/custom.test.ts (1)
2-2
: Consistent named import spacingLine 2 now matches the prevailing formatting style; no behavioral impact.
__tests__/phpcs.test.ts (1)
2-2
: Consistent named import spacingLine 2 aligns the import style with the rest of the suite; change is purely cosmetic and good to keep.
__tests__/phpmd.test.ts (1)
2-2
: Consistent named import spacingLine 2’s spacing tweak matches our formatter output and keeps tests stylistically uniform.
__tests__/phpstan.test.ts (1)
2-2
: Consistent named import spacingLine 2 now follows the standard spacing for named imports; looks good.
.gitattributes (1)
1-3
: Line-ending normalization is a solid additionLine 1 enforces LF endings repo-wide and Line 3 keeps dist as generated—helps CI workflows stay deterministic.
.licenses/npm/@actions/io.dep.yml (1)
1-20
: LGTM!The license manifest structure is correct and the MIT license text matches the standard template. The metadata appropriately documents the @actions/io dependency.
.gitignore (1)
1-102
: LGTM!The updated .gitignore provides comprehensive coverage of Node.js, TypeScript, and common tooling artifacts. Environment files are properly excluded for security, and the patterns align with GitHub's official Node.gitignore template.
.vscode/launch.json (1)
1-15
: LGTM!The VSCode launch configuration is properly structured for debugging GitHub Actions locally using the
@github/local-action
tool. The arguments and settings are appropriate.tsconfig.base.json (1)
1-23
: LGTM!The TypeScript configuration is well-structured with modern settings appropriate for a GitHub Action project:
- NodeNext module system for ESM support
- ES2022 target for recent Node.js versions
- Comprehensive strict flags for type safety
- Declaration output disabled (appropriate for applications)
.env.example (1)
1-61
: LGTM!The environment template provides excellent guidance with:
- Clear security warnings about not committing secrets
- Accurate documentation of GitHub Actions input format
- Comprehensive list of available environment variables
- Helpful comments and examples
.licenses/npm/tunnel.dep.yml (1)
1-35
: LGTM!The license manifest correctly documents the tunnel package with valid MIT license text. The dual license sources (LICENSE and README.md) provide comprehensive documentation.
.licenses/npm/undici.dep.yml (1)
1-34
: LGTM!The license manifest correctly documents the undici package with valid MIT license text and appropriate attribution to the Undici contributors.
This commit addresses several issues to improve the CI workflow and documentation: - **CI Workflow (`.github/workflows/ci.yml`):** - Removed redundant `milliseconds` input for the `test-action` job. - Added `php-version: 8.4` to the `test-action` job, ensuring compatibility with PHP 8.4 for testing purposes. - **Documentation (`README.md`):** - Corrected "Github" to "GitHub" for proper capitalization. - Updated wording to "Drupal codebase" for consistency. - Ensured code formatting for registry options (`ghcr` and `dockerhub`). - Clarified that `grumphp run` is executed on the "codebase". - Clarified that `phpstan` is run on the "codebase". - **Dependencies (`package.json`):** - Added `engines.node` to specify a minimum Node.js version of 24.0.0, ensuring compatibility with newer Node.js features used by the action.
This commit addresses several issues found in CI and completes the conversion of modules to ES Modules. Key changes include: - Updated `package.json` and `package-lock.json` to include `globals` and adjust Node.js engine requirements. - Removed unnecessary `main` field from `package.json`. - Modified import statements to use `.js` extension for ES Module compatibility. - Updated import paths for checks modules (e.g., `custom.js` instead of `custom`). - Refactored CI test command to use `NODE_OPTIONS=--experimental-vm-modules`. - Corrected a minor documentation typo in the `README.md`. - Updated the coverage badge to reflect a more accurate coverage percentage.
Updates the `runs.using` property in `action.yml` from `node20` to `node24` to utilize the latest Node.js runtime.
This commit introduces a new workflow for versioning and updates dependencies within the existing CI workflow. The versioning workflow has been configured with necessary permissions, and the CI workflow now ensures that the build job waits for other jobs to complete before executing.
Replaced manual `git add`, `git commit`, and `git push` commands with the `stefanzweifel/git-auto-commit-action` GitHub Action. This streamlines the process of committing updated license files when the workflow is triggered manually.
The `continue-on-error: true` property is added to the `test-action` job in the CI workflow. This prevents the workflow from failing if the local action test encountered an error, allowing the build process to continue.
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
package.json (1)
21-23
: Consider addingmain
field for broader compatibility.While the
exports
field is the modern ES module approach, adding amain
field pointing to the same entry point would improve compatibility with older Node.js versions and tools that don't yet support theexports
field.Apply this diff to add the
main
field:"all": "npm run format:write && npm run lint && npm run test && npm run coverage && npm run package" }, + "main": "./dist/index.js", "exports": { ".": "./dist/index.js" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
badges/coverage.svg
is excluded by!**/*.svg
dist/index.js
is excluded by!**/dist/**
dist/index.js.map
is excluded by!**/dist/**
,!**/*.map
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (13)
.github/workflows/ci.yml
(1 hunks).github/workflows/versioning.yml
(1 hunks)README.md
(2 hunks)__tests__/custom.test.ts
(2 hunks)__tests__/phpstan.test.ts
(1 hunks)action.yml
(1 hunks)package.json
(2 hunks)src/checks/custom.ts
(1 hunks)src/checks/grumphp.ts
(1 hunks)src/checks/phpstan.ts
(1 hunks)src/index.ts
(1 hunks)src/main.ts
(3 hunks)src/types.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/index.ts
- src/checks/phpstan.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/checks/custom.ts
- README.md
- src/main.ts
- tests/phpstan.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
__tests__/custom.test.ts (1)
src/checks/custom.ts (1)
custom
(3-9)
🔇 Additional comments (6)
.github/workflows/versioning.yml (1)
7-8
: LGTM!The
contents: write
permission is appropriate for this versioning workflow, which needs to push tags and update releases viaactions-tagger
.src/checks/grumphp.ts (1)
1-1
: LGTM!The explicit
.js
extension is required for ES module resolution, and the spacing adjustment improves consistency with project style conventions.__tests__/custom.test.ts (1)
2-2
: LGTM!The formatting improvements enhance readability without altering functionality.
Also applies to: 9-10, 19-20
package.json (1)
6-7
: LGTM on ES module migration and build system setup.The addition of
"type": "module"
and"private": true"
, along with the comprehensive script definitions using Rollup for bundling, establishes a solid modern build pipeline. The scripts cover formatting, linting, testing, coverage, and packaging appropriately.Also applies to: 8-20
.github/workflows/ci.yml (1)
1-93
: CI: action.yml defines php-version default '8.2' — confirm PHP 8.4 supportaction.yml declares an input
php-version
(default: "8.2"), sophp-version: 8.4
in the workflow is a valid input override; verify the action implementation and any used tooling actually support PHP 8.4 and update action.yml/defaults/docs if needed.src/types.ts (1)
1-1
: OptionsObject type change is semantically correct. The change from{}
toobject
better represents the intended constraint for option objects. Verify that no code—particularly where checks are invoked inmain.ts
—passes primitives as theoptions
argument.
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
badges/coverage.svg
is excluded by!**/*.svg
dist/index.js
is excluded by!**/dist/**
dist/index.js.map
is excluded by!**/dist/**
,!**/*.map
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (14)
.github/workflows/ci.yml
(1 hunks).github/workflows/licensed.yml
(1 hunks).github/workflows/versioning.yml
(1 hunks)README.md
(2 hunks)__tests__/custom.test.ts
(2 hunks)__tests__/phpstan.test.ts
(1 hunks)action.yml
(1 hunks)package.json
(2 hunks)src/checks/custom.ts
(1 hunks)src/checks/grumphp.ts
(1 hunks)src/checks/phpstan.ts
(1 hunks)src/index.ts
(1 hunks)src/main.ts
(3 hunks)src/types.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/ci.yml
- package.json
- tests/custom.test.ts
- action.yml
- .github/workflows/licensed.yml
- tests/phpstan.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/index.ts (1)
src/main.ts (1)
run
(25-105)
src/main.ts (1)
dist/index.js (1)
env
(34230-34230)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint Codebase
🔇 Additional comments (2)
.github/workflows/versioning.yml (1)
7-8
: Write permission is appropriate here.
Actions-R-Us/actions-tagger
needscontents: write
to push thelatest
tag, so adding this block keeps the workflow functional while remaining scoped.src/main.ts (1)
5-12
: LGTM! ES module imports are correctly configured.The explicit
.js
extensions on relative imports are correct for ES modules in TypeScript. This aligns with the project-wide migration to ES modules.
import { run } from './main.js' | ||
|
||
/* istanbul ignore next */ | ||
run() |
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.
Handle run()
rejection to fail the action cleanly
run()
is async and can throw (e.g., invalid inputs). Calling it without a rejection handler turns those into unhandled promise rejections, so the action fails with a noisy stack trace instead of a clear core.setFailed
message. Please import @actions/core
here and catch the rejection so failures surface consistently.
-import { run } from './main.js'
-
-/* istanbul ignore next */
-run()
+import * as core from '@actions/core'
+import { run } from './main.js'
+
+/* istanbul ignore next */
+void run().catch(error => {
+ if (error instanceof Error) {
+ core.setFailed(error.message)
+ } else {
+ core.setFailed(String(error))
+ }
+})
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { run } from './main.js' | |
/* istanbul ignore next */ | |
run() | |
import * as core from '@actions/core' | |
import { run } from './main.js' | |
/* istanbul ignore next */ | |
void run().catch(error => { | |
if (error instanceof Error) { | |
core.setFailed(error.message) | |
} else { | |
core.setFailed(String(error)) | |
} | |
}) |
🤖 Prompt for AI Agents
In src/index.ts around lines 5 to 8, the async run() call is unhandled so
rejections become unhandled promise rejections; import @actions/core and attach
a rejection handler to fail the action cleanly (e.g., import core from
'@actions/core' and replace the bare run() call with run().catch(err =>
core.setFailed(String(err))) so any thrown errors are reported via
core.setFailed).
} | ||
|
||
async function run(): Promise<void> { | ||
export async function run(): Promise<void> { |
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Good refactor! Exporting run()
improves testability.
Making run()
a named export improves modularity and makes the function testable in isolation. Ensure that tests exist for this function to cover the main action logic.
Run the following script to verify test coverage for the run()
function:
🏁 Script executed:
#!/bin/bash
# Description: Check if tests exist for the run() function
# Search for test files that import or test the run function
rg -n --type=ts -C3 "run\(" __tests__/ || echo "No tests found for run() function"
# Check if main.ts is imported in test files
rg -n --type=ts "from.*main" __tests__/ || echo "No imports of main.ts found in tests"
Length of output: 70
Add tests for the exported run()
function
No tests currently import or invoke run()
from src/main.ts
; add a test file (e.g., __tests__/main.spec.ts
) to cover its core logic.
🤖 Prompt for AI Agents
In src/main.ts at line 25, the exported async function run() lacks test
coverage. Create a new test file named __tests__/main.spec.ts and write tests
that import and invoke the run() function to verify its core logic. Make sure
the tests cover various scenarios for run() to ensure it behaves as expected.
This commit introduces a comprehensive CI setup for the project, along with essential linting and configuration files. Key changes include:
ci.yml
: Configures a complete continuous integration pipeline including TypeScript tests, action testing, and build steps.check-dist.yml
: Adds a workflow to ensure thedist/
directory contains the expected transpiled code.codeql-analysis.yml
: Integrates CodeQL for static analysis.licensed.yml
: Sets up dependency license checking using the Licensed tool.linter.yml
: Utilizes Super-linter for comprehensive codebase linting..env.example
: Provides an example environment file..eslinlint.yml
,.markdown-lint.yml
,.prettierignore
,.prettierrc.yml
,.yaml-lint.yml
: Standard configuration files for ESLint, markdownlint, Prettier, and yamllint, ensuring code consistency..vscode/launch.json
,.vscode/mcp.json
,.vscode/settings.json
: VS Code configuration for debugging and workspace settings..licensed.yml
,.node-version
: Configuration for the Licensed tool and Node.js version.actionlint.yml
: Configuration for Actionlint.rollup.config.ts
: Rollup configuration for bundling.tsconfig.base.json
,tsconfig.eslint.json
,tsconfig.json
: TypeScript configuration files for base settings, ESLint, and the main project..gitignore
: Updated to ignore built artifacts and local environment files.__tests__
) refactored for consistency.action.yml
: Updated with refined descriptions and defaults.These changes establish a robust foundation for code quality, automated testing, and streamlined development workflows.
Summary by CodeRabbit