Adjust lint script to allow warnings without failing CI #492
Conversation
Deploying jasaweb with
|
| Latest commit: |
6af9f20
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://42e9ed08.jasaweb-dhd.pages.dev |
| Branch Preview URL: | https://2025-11-23.jasaweb-dhd.pages.dev |
Summary of ChangesHello @sulhicmz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the project's linting configuration to prevent CI failures that are solely triggered by linting warnings. By relaxing the strict warning threshold, the linting process will now provide feedback on warnings without blocking the build, aiming to improve developer workflow efficiency while still highlighting potential code quality issues. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the --max-warnings 0 flag from the lint script to allow ESLint warnings without failing CI. While this achieves the stated goal, it introduces a significant risk by allowing security-related warnings to be ignored by the CI pipeline, and it also breaks the --strict mode functionality in your quality check script. My review includes a suggestion to create a separate, strict linting script for CI and pre-commit hooks to mitigate this risk while still providing flexibility for local development.
| "test:e2e": "playwright test", | ||
| "test:api": "cd apps/api && pnpm test", | ||
| "lint": "eslint . --max-warnings 0", | ||
| "lint": "eslint .", |
There was a problem hiding this comment.
This change removes the --max-warnings 0 flag, which means the linting process will no longer fail on warnings. While this may be intended to prevent CI failures for non-critical issues, it has some significant drawbacks:
- Security Risks: Your ESLint configuration (
eslint.config.js) includes several security-related rules (e.g.,security/detect-object-injection,security/detect-child-process) that are set towarn. By allowing warnings, you risk introducing security vulnerabilities that will not be flagged as errors in your CI pipeline. - Broken Strict Mode: The
scripts/dev-tools/code-quality.shscript includes a--strictmode, which is used by thepre-commitcommand. This mode is intended to fail on warnings. With this change, thepnpm lintcommand will always succeed if there are only warnings, rendering the--strictflag ineffective for linting.
A safer approach is to maintain a strict linting step for CI and pre-commit hooks while allowing a more lenient one for local development. I suggest adding a separate script for strict linting. You can then update code-quality.sh to use this new script when --strict mode is enabled.
| "lint": "eslint .", | |
| "lint": "eslint .", | |
| "lint:strict": "eslint . --max-warnings 0", |
- Add lint:strict script that enforces zero warnings - Keep regular lint script to allow warnings in development
github-actions
bot
force-pushed
the
2025-11-23
branch
from
344f1bc to
8ebf10e
Compare
Summary of Actions Taken
The changes address the security concerns raised in the review while maintaining the intended functionality of allowing warnings during development. |
PR Status Update✅ Changes Made: Successfully addressed all review comments by adding the ✅ Code Quality: The changes maintain security checks while allowing development flexibility. ❌ Current Blocker: The 'opencode' status check is failing, preventing automatic merge. The PR is ready for merge once the opencode check issue is resolved. The changes made fulfill the requirements from the code review. |
Summary
Testing
Codex Task