-
Notifications
You must be signed in to change notification settings - Fork 39
refactor: add more consts and use them #219
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
refactor: add more consts and use them #219
Conversation
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.
Pull request overview
This PR refactors the codebase to introduce and use string constants for validation types, making the code more maintainable and enabling safer type-based switching in client code.
Changes:
- Added
SecurityValidationandDocumentValidationconstants tohelpers/constants.go - Replaced hardcoded validation type strings with constants throughout the codebase
- Updated test files to use the new constants for consistency
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| helpers/constants.go | Adds two new exported constants: SecurityValidation and DocumentValidation |
| validator.go | Updates document validation error to use helpers.DocumentValidation constant |
| validator_test.go | Replaces hardcoded validation type strings with constants in test assertions and error creation |
| schema_validation/validate_document.go | Updates schema validation errors to use helpers.Schema constant |
| schema_validation/validate_document_test.go | Updates test to use helpers.Schema constant |
| parameters/validate_security.go | Replaces hardcoded "security" strings with helpers.SecurityValidation constant |
| errors/validation_error_test.go | Updates tests to use helpers.ParameterValidationPath constant and adds helpers import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #219 +/- ##
=======================================
Coverage 97.58% 97.58%
=======================================
Files 56 56
Lines 5176 5176
=======================================
Hits 5051 5051
Misses 125 125
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a0d862a to
e8de77e
Compare
|
LGTM, thanks! |
daveshanley
left a comment
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.
nice.
What
This PR doesn't update any actual logic.
Here's a list of changes:
helpers.SecurityValidationand update the code to use ithelpers.DocumentValidationand update the code to use ithelpers.PathValidationand separate it fromParameterValidationPathsince they are technically a different things.helpers.ValidationMissingand update the code to use ithelpers.ValidationMissingOperationand update the code to use it.RequestMissingOperation. UsingRequestMissingOperationwill trigger a linter error such as this one.Why
This is to make it safer to use the library by being able to write code such as: