fix: add missing sqlstring deps #10
Conversation
WalkthroughThis pull request removes the CodeQL GitHub Actions workflow and updates the Node.js workflow to support additional Node.js and MySQL versions while adding a secret. The README now includes a "For the local dev" section with commands to start a MySQL service, run tests, and stop the service. A new Docker Compose file for a MySQL service is introduced, and package dependencies are updated along with a change in the test configuration to use nullish coalescing for database parameters. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant DC as Docker Compose
participant MySQL as MySQL Service
participant Test as Test Runner
Dev->>DC: Run "docker compose up -d"
DC->>MySQL: Launch MySQL container (MySQL v9)
Note right of MySQL: Wait ~20 seconds for initialization
Dev->>Test: Execute "npm test"
Test-->>Dev: Return test results
Dev->>DC: Run "docker compose down"
DC->>MySQL: Stop MySQL container
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
test/config.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-eggache". (The package "eslint-plugin-eggache" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-eggache" was referenced from the config file in ".eslintrc » eslint-config-egg/typescript » ./index.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
PR Overview
This PR aims to fix missing sqlstring dependencies and improve local development and CI configurations.
- Introduces a Docker Compose configuration for a MySQL service for local testing.
- Updates README.md with instructions for starting, stopping, and testing against the local MySQL service.
- Enhances the CI workflow by expanding supported Node.js and MySQL versions and adding CODECOV token configuration.
Reviewed Changes
| File | Description |
|---|---|
| docker-compose.yml | New Docker Compose configuration for a MySQL service |
| README.md | Added instructions for running a local MySQL service and executing unit tests |
| .github/workflows/nodejs.yml | Updated CI workflow with additional Node.js and MySQL versions, and inclusion of CODECOV token |
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
| name: eggjs_rds_dev_services_mysql | ||
|
|
There was a problem hiding this comment.
The 'name' key is not a standard Docker Compose property; consider removing it or replacing it with a valid configuration option such as using the '-p' flag when starting compose.
| name: eggjs_rds_dev_services_mysql |
|
Removed dependencies detected. Learn more about Socket for GitHub ↗︎ 🚮 Removed packages: npm/git-contributor@2.1.5 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #10 +/- ##
==========================================
- Coverage 99.25% 98.44% -0.82%
==========================================
Files 9 10 +1
Lines 1078 1159 +81
Branches 223 238 +15
==========================================
+ Hits 1070 1141 +71
- Misses 8 18 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
package.json (1)
11-11: 💡 Verification agent🧩 Analysis chain
Potential issue: missing dependency for script.
There's a script that references
git-contributor, but according to the AI summary, this dependency has been removed from devDependencies. This will causenpm run contributorto fail.
🏁 Script executed:
#!/bin/bash # Check if git-contributor is still in the package.json cat package.json | grep -A 10 "devDependencies" | grep "git-contributor" # Check if the script works without the dependency npm list git-contributor || echo "git-contributor not found in node_modules"Length of output: 239
Action Required: Update NPM script or add missing dependency
The executed checks confirm that the
git-contributordependency is absent from both thedevDependenciesandnode_modules, meaning that runningnpm run contributorwill indeed fail. Please update this reference by either re-adding the dependency to yourdevDependencies(ordependencies) or removing/adjusting the script if it is no longer required.
- File: package.json, line 11:
"contributor": "git-contributor",
♻️ Duplicate comments (1)
docker-compose.yml (1)
1-2:⚠️ Potential issueRemove the non-standard 'name' property.
The 'name' key is not a standard Docker Compose property. Consider removing it or use the '-p' flag when starting compose instead.
-name: eggjs_rds_dev_services_mysql -
🧹 Nitpick comments (2)
docker-compose.yml (1)
7-9: Improve environment variable configuration.The current setup allows an empty password which is not secure for production. This might be acceptable for local development, but consider adding a comment to clarify this is for development purposes only.
environment: MYSQL_ROOT_PASSWORD: ${MYSQL_ROOT_PASSWORD:-} MYSQL_ALLOW_EMPTY_PASSWORD: yes + # Note: Empty password allowed for local development only MYSQL_DATABASE: ${MYSQL_DATABASE:-test}README.md (1)
436-456: Refine Local Development Instructions for Clarity
The new "For the local dev" section is a great addition for setting up a local MySQL service. Consider rephrasing a few parts for improved clarity and consistency:
- Change "Run docker compose to start test mysql service" to "Run docker compose to start the test MySQL service".
- In the bash block, update the comment "if you run the first time, should wait for ~20s to let mysql service init started" to something like "If running for the first time, please wait ~20 seconds to allow the MySQL service to initialize."
These small adjustments can enhance readability and maintain a professional tone.🧰 Tools
🪛 LanguageTool
[uncategorized] ~451-~451: This verb may not be in the correct form. Consider using a different form for this context.
Context: ... unit testsbash npm testStop test mysql service ```bash docker compose -...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/codeql.yml(0 hunks).github/workflows/nodejs.yml(1 hunks)README.md(1 hunks)docker-compose.yml(1 hunks)package.json(1 hunks)test/config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/codeql.yml
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~451-~451: This verb may not be in the correct form. Consider using a different form for this context.
Context: ... unit tests bash npm test Stop test mysql service ```bash docker compose -...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
🔇 Additional comments (5)
package.json (1)
20-21: Dependencies added correctly.The addition of
sqlstringas a dependency aligns with the PR title "fix: add missing sqlstring deps". This is a good fix as sqlstring is necessary for properly escaping SQL queries to prevent SQL injection attacks.docker-compose.yml (1)
12-16: Good implementation of healthcheck.The healthcheck configuration is well-defined with appropriate interval, timeout, and retry settings to ensure the MySQL service is properly monitored.
test/config.ts (1)
2-6: Good use of nullish coalescing operator.Replacing
||with??is a good practice here as it allows falsy values like empty strings to be valid configuration values while still providing defaults for null/undefined. This is especially relevant for database passwords which could legitimately be empty strings..github/workflows/nodejs.yml (1)
17-18: Good security practice adding CODECOV_TOKEN as a secret.Including the CODECOV_TOKEN as a GitHub secret is the proper way to handle sensitive tokens in CI workflows.
README.md (1)
463-466: Dynamic Contributors Section Update
Switching to a dynamic contributor image using contrib.rocks is an excellent move for automating contributor updates. Just ensure that the external service is reliable and that its update frequency aligns with your project's needs.
|
|
||
| services: | ||
| mysql: | ||
| image: mysql:9 |
There was a problem hiding this comment.
Invalid MySQL version specified.
MySQL 9 does not exist yet. The latest MySQL version is 8.x (as of March 2025). This will cause the Docker container to fail.
- image: mysql:9
+ image: mysql:8📝 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.
| image: mysql:9 | |
| image: mysql:8 |
| version: '16, 18, 20, 22' | ||
| mysql_version: '5, 8, 9' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Node.js and MySQL version expansion.
Adding Node.js 22 and MySQL 9 for testing is forward-looking, but be aware that MySQL 9 doesn't exist yet - this will cause test failures for that matrix entry.
version: '16, 18, 20, 22'
- mysql_version: '5, 8, 9'
+ mysql_version: '5, 8'🌐 Web query:
What is the latest stable version of MySQL available?
💡 Result:
The latest stable versions of MySQL depend on the release track chosen, with distinct options available for Long-Term Support (LTS) and Innovation tracks as of March 2025:
LTS Track (Stability-Focused)
- MySQL 8.4.1 LTS: Released in July 2024, this version is part of the LTS track, providing stability with essential bug fixes and security updates. It is supported under Oracle’s Lifetime Support Policy, including 5 years of premier support[5][9].
Innovation Track (Feature-Rich)
- MySQL 9.0.0 Innovation: Also released in July 2024, this version introduces new features and improvements for developers in fast-paced environments. Innovation releases are supported until the next Innovation version becomes available[2][5][9].
Key Details:
- AWS RDS Support: The latest MySQL version on Amazon RDS is 8.0.41, released on February 19, 2025, with standard support until March 2026[1].
- Versioning Structure:
- LTS (e.g., 8.4.x): Focuses on stability and extended support.
- Innovation (e.g., 9.x.x): Prioritizes new features and frequent updates[2][5].
- Deprecation Note: MySQL 8.0.x versions are being phased out, with MySQL Installer discontinued after 8.0.41[4].
For most production environments requiring stability, MySQL 8.4.1 LTS is recommended. Developers seeking cutting-edge features should opt for MySQL 9.0.0 Innovation[5][9].
Citations:
- 1: https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/MySQL.Concepts.VersionMgmt.html
- 2: https://docs.oracle.com/en-us/iaas/mysql-database/doc/mysql-server-versions.html
- 3: https://en.wikipedia.org/wiki/MySQL
- 4: https://dev.mysql.com/downloads/installer/
- 5: https://dev.mysql.com/doc/refman/en/mysql-releases.html
- 6: https://dev.mysql.com/doc/relnotes/mysql/8.0/en/
- 7: https://dev.mysql.com/doc/relnotes/mysql/8.4/en/
- 8: https://www.mysql.com/downloads/
- 9: https://blogs.oracle.com/mysql/post/mysql-july-2024-ga-releases-now-available
- 10: https://endoflife.date/mysql
Action: Remove the unsupported MySQL version from the testing matrix
While including Node.js 22 is forward-looking, keep in mind that—even though MySQL 9.0.0 is now available as an Innovation release—our testing should focus on production-ready, stable releases. For stability (e.g., MySQL 8.4.1 LTS for most production environments), it is best to limit the matrix to versions 5 and 8. Please update the workflow file accordingly:
- File:
.github/workflows/nodejs.yml(Lines 15-16) - Required Change:
version: '16, 18, 20, 22' - mysql_version: '5, 8, 9' + mysql_version: '5, 8'
📝 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.
| version: '16, 18, 20, 22' | |
| mysql_version: '5, 8, 9' | |
| version: '16, 18, 20, 22' | |
| mysql_version: '5, 8' |
[skip ci] ## [1.2.2](v1.2.1...v1.2.2) (2025-03-09) ### Bug Fixes * add missing sqlstring deps ([#10](#10)) ([7339921](7339921))
Summary by CodeRabbit
Documentation
New Features
Chores