-
-
Notifications
You must be signed in to change notification settings - Fork 11
feat: use ajv for json-schema validation #172
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
|
""" WalkthroughThe changes update the configuration validation logic by switching from the Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant ConfigLoader
participant AjvValidator
CLI->>ConfigLoader: loadTolgeeRc(configPath)
ConfigLoader->>AjvValidator: Compile schema (async)
ConfigLoader->>AjvValidator: Validate config
AjvValidator-->>ConfigLoader: Validation result (valid/invalid)
alt Validation fails
ConfigLoader->>CLI: exitWithError(formatted error message)
else Validation succeeds
ConfigLoader-->>CLI: Return parsed config
end
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
npm warn config production Use 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/config/tolgeerc.ts (1)
112-114: Consider simplifying validation check.The validation and error checking could be slightly simplified.
- validate(config); - const firstErr = validate.errors?.[0]; - - if (firstErr) { + if (!validate(config)) { + const firstErr = validate.errors?.[0];This would combine the validation call with the error check, which is a common pattern when using Ajv.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json(1 hunks)src/config/tolgeerc.ts(3 hunks)test/e2e/config.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/e2e/config.test.ts (2)
test/e2e/utils/tmp.ts (1)
createTmpFolderWithConfig(27-33)test/e2e/utils/run.ts (1)
run(98-105)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: E2E Tests (ubuntu-latest, 22)
- GitHub Check: E2E Tests (ubuntu-latest, 20)
- GitHub Check: E2E Tests (ubuntu-latest, 18)
🔇 Additional comments (10)
package.json (2)
33-33: Dependencies updated appropriately.Adding the Ajv library as a dependency aligns with the PR objective of switching to ajv for JSON-schema validation.
39-39: Upgraded jsonschema version.Upgrading the jsonschema dependency to a newer version is a good practice, especially when making validation-related changes.
test/e2e/config.test.ts (5)
4-13: Well-structured test for format validation.This test correctly verifies that the schema validation fails when an invalid format is provided. The test creates a temporary config file with an invalid value, runs the CLI command, and checks both the exit code and error message.
15-23: Good coverage for projectId validation.This test ensures that the schema validation correctly handles invalid projectId values. The use of an object instead of a number is a good edge case to test.
25-33: Parser validation test looks good.This test verifies that the schema validation correctly handles invalid parser values, ensuring that only supported parsers can be specified in the configuration.
35-45: Nested property validation test is valuable.Testing validation for nested properties like push.forceMode ensures that the schema validation works for all levels of the configuration object.
47-54: Excellent test for future extensibility.This test is particularly valuable as it verifies that unknown properties don't cause validation failures. This ensures backward compatibility if new properties are added to the schema in future versions.
src/config/tolgeerc.ts (3)
11-11: Import updated to use Ajv.The import has been correctly updated to use Ajv instead of the previous jsonschema Validator.
34-34: Minor string literal adjustment.The quote style has been updated from single quotes to a mix of double and single quotes to improve readability of the error message.
109-118: Validation logic switched to Ajv.The validation logic has been successfully migrated from jsonschema to Ajv. Using
allowUnionTypes: trueis a good choice to ensure compatibility with schemas that use union types.The error message formatting is improved, with a clean approach to convert JSON paths (with slashes) to dot notation, which is more readable for users.
63aa835 to
dfb9959
Compare
|
🎉 This PR is included in version 2.12.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary by CodeRabbit